[lkml]   [2000]   [Sep]   [18]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    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
    Please read the FAQ at

     \ /
      Last update: 2005-03-22 12:38    [W:0.023 / U:6.560 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site