[lkml]   [2012]   [May]   [10]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH v2] compat: Fix RT signal mask corruption via sigprocmask
    On Thu, May 10, 2012 at 6:04 AM, Jan Kiszka <> wrote:
    > +               case SIG_BLOCK:
    > +                       sigaddsetmask(&new_blocked, new_set);
    > +                       break;
    > +               case SIG_UNBLOCK:
    > +                       sigdelsetmask(&new_blocked, new_set);
    > +                       break;

    Ok, I think SIG_[UN]BLOCK are now clearly right. However:

    > +               case SIG_SETMASK:
    > +                       new_blocked.sig[0] &=
    > +                               ~((old_sigset_t)(compat_old_sigset_t)-1);
    > +                       new_blocked.sig[0] |= new_set;
    > +                       break;

    I don't think this is clear.

    The semantics for the *native* SIG_SETMASK has been to only change the
    lower word of the sigset_t.

    And that was actually defined in terms of "compat_sigset_word", not

    Now, they are both generally the same, and so I think your code does
    the right thing, but I have to say that I really had to look closely
    to make sure that yes, your code was right.

    Anyway, my *gut* feel is that it would be much clearer to write the above as

    compat_sigset_word x = new_set;
    memcpy(new_blocked.sig, &x, sizeof(x));

    together with a comment to the effect that sigprocmask(SIG_SETMASK..)
    only changes the first word of the structure.

    That said, I think your patch does look technically correct, so maybe
    it's just me who thinks it is very non-obvious and hard to read.

    The memcpy approach will also generate better code. This is the "mask-and-set":

    movabsq $-4294967296, %rax #, tmp89
    andq -32(%rbp), %rax # new_blocked.sig, tmp89
    orq %rdx, %rax # new_set, tmp89
    movq %rax, -32(%rbp) # tmp89, new_blocked.sig

    and this is the memcpy:

    movl %edx, -32(%rbp) # new_set,

    ie it is done as a simple 32-bit store.

    I think I'll just edit your patch directly, no need to send me a new version.

    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to
    More majordomo info at
    Please read the FAQ at

     \ /
      Last update: 2012-05-10 18:21    [W:0.025 / U:94.596 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site