Errors in sigaction struct definition

From: Jordi Brinquez
Date: Thu Mar 10 2005 - 13:55:43 EST


I found a problem while using sigaction structure because of problems
on definition of that structure.

I found it on version 2.6.10 but it was confirmed on version 2.6.8 and
2.6.11 (so probably on other 2.6.x versions)

Extract from /include/asm-i386/signal.h (lines 142-172)

#ifdef __KERNEL__
struct old_sigaction {
__sighandler_t sa_handler;
old_sigset_t sa_mask;
unsigned long sa_flags;
__sigrestore_t sa_restorer;
};

struct sigaction {
__sighandler_t sa_handler;
unsigned long sa_flags;
__sigrestore_t sa_restorer;
sigset_t sa_mask; /* mask last for extensibility */
};

struct k_sigaction {
struct sigaction sa;
};
#else
/* Here we must cater to libcs that poke about in kernel headers. */

struct sigaction {
union {
__sighandler_t _sa_handler;
void (*_sa_sigaction)(int, struct siginfo *, void *);
} _u;
sigset_t sa_mask;
unsigned long sa_flags;
void (*sa_restorer)(void);
};

As you can see the order of the fields in sigaction struct defined
under __KERNEL__ is:
sa_handler;
sa_flags;
sa_restorer;
sa_mask;
and the order of the fields of the section for the user code is:
union {...} _u;
sa_mask;
sa_flags;
sa_restorer;

The order is not the same.

Now if we look at the routine that manages the sigaction
(rt_sigaction) we have the following code:

Extract from /kernel/signal.c (lines 2545-2573)

#ifdef __ARCH_WANT_SYS_RT_SIGACTION
asmlinkage long
sys_rt_sigaction(int sig,
const struct sigaction __user *act,
struct sigaction __user *oact,
size_t sigsetsize)
{
struct k_sigaction new_sa, old_sa;
int ret = -EINVAL;

/* XXX: Don't preclude handling different sized sigset_t's. */
if (sigsetsize != sizeof(sigset_t))
goto out;

if (act) {
if (copy_from_user(&new_sa.sa, act, sizeof(new_sa.sa)))
return -EFAULT;
}

ret = do_sigaction(sig, act ? &new_sa : NULL, oact ? &old_sa : NULL);

if (!ret && oact) {
if (copy_to_user(oact, &old_sa.sa, sizeof(old_sa.sa)))
return -EFAULT;
}
out:
return ret;
}
#endif /* __ARCH_WANT_SYS_RT_SIGACTION */

As you can see the algorithm that copies the values from user struct
to kernel struct is copy_from_user (and copy to_user) so because the
diferent definition of the structures the data goes corrupted.

To solve that problem there are two solutions:

- Reorder the fields on user structure (easy solution)
- Change the copy_to_user and copy_from_user for code using __put_user
and __get_user

I think that implementing both can prevent future problems.

Greets,

Jordi
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/