lkml.org 
[lkml]   [2000]   [Sep]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [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/

\
 
 \ /
  Last update: 2005-03-22 12:38    [W:0.068 / U:0.176 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site