[PATCH] Security fixes for rt signals, siginfo posting

From: Jakub Jelinek (jakub@redhat.com)
Date: Wed Jan 19 2000 - 08:00:44 EST


Hi!

I've seen posting of siginfo for non-rt signals made it into the kernel.
That's great news.
I'd like to move things a little bit further in the siginfo area.
Currently, the kernel has a bunch of kernel stack leaks to userland which
are a serious security issue. Basically, if one runs rgrep 'si_code.*=' in
the kernel tree, all but sys_kill are broken.
One solution for that is to put a memset(&info, 0, sizeof(siginfo_t));
before every force_sig_info/send_sig_info and related siginfo structure
setup. But sizeof(siginfo_t) is 128 and in my tree there are more than 100
places which would need the memset.

Instead of that, I've coded a solution which speeds up siginfo handling both
in the kernel and when copying to userland by only copying what's needed
from the siginfo structure and not always the whole 128 byte structure (each
is copied 3 times - once into the signal queue, once in dequeue_signal and
once into userland). So it makes a little bit difference whether you're
moving 20 or 128 bytes of data.

Userland generated siginfo is always moved as a 128 chunk of data, while
kernel generated siginfo gets some bits of si_code filled with the
identification on what siginfo union members are actually used.
When copying siginfo to userland, those bits get sign extended from the 15th
bit, so userland does not see any difference.

There are two architecture specific routines, copy_siginfo and
copy_siginfo_to_user, which actually move siginfo around and ports are free
to decide how to implement them, provided that the copy_siginfo_to_user does
not leak any data which is should not be set for a particular union type and
clears the internal bits of si_code. So this patch basically tries to avoid
current and future kernel stack leaks into userland, if the author obeys
these rules:

Before calling send_sig_info/force_sig_info, you should either:
a) play safe and bzero the whole structure before setting up fields
b) fill si_signo, si_errno and si_code
   and, depending on si_code, some other fields
   - if si_code is < 0, then the whole structure must be cleared first
   - if si_code is __SI_FAULT style (ILL_*, SEGV_*, BUS_*, TRAP_*, FPE_*, EMT_*),
     then si_addr must be filled (and si_trapno as well on sparc/sparc64)
   - if si_code is __SI_KILL style (the most common, e.g. SI_USER, SI_KERNEL),
     then si_uid and si_pid must be filled
   - if si_code is __SI_POLL style (POLL_*), then si_fd and si_band must be
     filled
   - if si_code is __SI_CHLD style (CLD_*), then si_pid, si_uid16 (using SET_SIGINFO_UID16),
     si_cld_uid (this is unfortunate due to uid32 changes), si_status,
     si_utime and si_stime must be filled

This has an additional advantage for multiple ABI architectures
(sparc64(sparc), ia64(ia32), ...) that the kernel knows what the semantics
of a particular siginfo structure is and can convert it correctly
(previously, sparc64 used to convert e.g. based on signr, but that's very
wrong, e.g. you can call notify_parent with any signal you like and it would
work only for SIGCLD and not for the other signals).

In addition to this, this patch adds a bunch of info for kernel generated
fault signals (on i386 just a few - there should be more added, while
sparc/sparc64 should be more less complete).
A lot of people are crying for actually usable siginfo which is only when
kernel generates that information.
Plus this patch as per rth's suggestion, it does not check if a non-rt signal is still
pending in the queue because it is never.

This patch has been so far tested on i386 and sparc64 (actually, it is
implemented for i386,sparc,sparc64 and maintainers should fix the remaining
ports).

BTW: When hacking on the patch, I found that on i386 glibc defines siginfo_t
to have a 32bit uid in the place of kernel's 16bit (but the kernel has
padding there), which leads me to the question: is it really necessary to
keep _kill._uid on intel with 16bit type and introduce _kill._uid32?
Having _kill._uid32 has serious drawbacks, because now the info->si_uid no
longer works for _sigchld and _rt union members.

Please apply.

Cheers,
    Jakub
___________________________________________________________________
Jakub Jelinek | jakub@redhat.com | http://sunsite.mff.cuni.cz/~jj
Linux version 2.3.40 on a sparc64 machine (1343.49 BogoMips)
___________________________________________________________________

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



This archive was generated by hypermail 2b29 : Sun Jan 23 2000 - 21:00:20 EST