Messages in this thread |  | | Date | Mon, 18 Sep 2000 20:56:58 +0200 | From | Jamie Lokier <> | Subject | Re: [PATCH] Fix queued SIGIO |
| |
Linus, please think this over before applying Andi's patch.
Andi Kleen wrote: > The problem is really that SI_SIGIO is negative, but it should be positive > to make SI_FROMUSER return false on it.
This is an old problem. There was a thread on this topic last March. Look for "accept() improvements for rt signals".
SI_SIGIO is not the only signal that's broken: SI_ASYNCIO, SI_TIMER and SI_MESGQ have the same problem. When those signals are used you'll be adding more and more exceptions to the SI_FROMUSER macro.
(For example the POSIX timers patch actually does exactly the same as Andi's patch, but for SI_TIMER instead of SI_SIGIO).
It's obvious that SI_SIGIO, SI_ASYNCIO, SI_TIMER and SI_MESGQ should all return false from SI_FROMUSER, assuming SI_FROMUSER is the right thing to use in any case.
> Changing it would unfortunately break binary compatibility. This patch > instead changes the definition of SI_FROMUSER/KERNEL to check explicitely > for SI_SIGIO and make it appear like a kernel generated signal type. This > prevents send_sig_info from looking at current .
That looks like major band-aid. What does SI_FROMUSER mean anyway?
I looked it up on the web. It doesn't appear in manual pages on the web in general, and I didn't find it in Single Unix. Let's investigate. /* * si_code values * Digital reserves positive values for kernel-generated signals. */
Hmm... Inherited from Digital Unix perhaps? Let's try Digital UNIX V4.0F... /usr/include/sys/siginfo.h:
/* negative si_codes are reserved for user-generated signals */ #define SI_QUEUE -1 /* sent by sigqueue */ #define SI_USER 0 /* sent by kill, sigsend, raise, etc. */ #define SI_NOINFO 1 /* unknown source */ #define SI_TIMER 0x10 /* sent by timer expiration */ #define SI_ASYNCIO 0x20 /* sent by AIO completion */ #define SI_MESGQ 0x40 /* sent by realtime mesq state transition */
#define SI_FROMUSER(siptr) ((siptr)->si_code <= 0) #define SI_FROMKERNEL(siptr) ((siptr)->si_code > 0)
Looks like DEC got this right, Linux blindly copied some of the definitions and made up some of the others (by setting SI_TIMER etc. to negative values but keeping the same definition of SI_FROMUSER).
_Something_ of binary compatibility will have to be broken to fix this problem. Either change the SI_TIMER etc. signal numbers, or change the SI_FROMUSER macro. Both of can potentially break applications. Changing SI_SIGINFO would obviously be the most serious.
I'd posit that changing SI_FROMUSER is the right fix. But changed to include SI_TIMER, SI_MESGQ and SI_ASYNCIO as well.
Andi says:
> It'll break programs that try to send SI_SIGIO (=-5) signals from userspace, > but I think that is ok.
Actually rt_sigqueueinfo has this test hard-coded in it:
if (info.si_code >= 0) return -EPERM;
with a comment "not even root is allowed to send signals from the kernel". Changing SI_FROMUSER won't affect this.
Perhaps it should. Do we consider SI_SIGIO, SI_TIMER etc. to be in the "not even root is allowed" category or not?
have a nice day, -- Jamie - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/
|  |