lkml.org 
[lkml]   [2020]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 4.4 035/100] futex: futex_wake_op, do not fail on invalid op
    Date
    From: Jiri Slaby <jslaby@suse.cz>

    commit e78c38f6bdd900b2ad9ac9df8eff58b745dc5b3c upstream.

    In commit 30d6e0a4190d ("futex: Remove duplicated code and fix undefined
    behaviour"), I let FUTEX_WAKE_OP to fail on invalid op. Namely when op
    should be considered as shift and the shift is out of range (< 0 or > 31).

    But strace's test suite does this madness:

    futex(0x7fabd78bcffc, 0x5, 0xfacefeed, 0xb, 0x7fabd78bcffc, 0xa0caffee);
    futex(0x7fabd78bcffc, 0x5, 0xfacefeed, 0xb, 0x7fabd78bcffc, 0xbadfaced);
    futex(0x7fabd78bcffc, 0x5, 0xfacefeed, 0xb, 0x7fabd78bcffc, 0xffffffff);

    When I pick the first 0xa0caffee, it decodes as:

    0x80000000 & 0xa0caffee: oparg is shift
    0x70000000 & 0xa0caffee: op is FUTEX_OP_OR
    0x0f000000 & 0xa0caffee: cmp is FUTEX_OP_CMP_EQ
    0x00fff000 & 0xa0caffee: oparg is sign-extended 0xcaf = -849
    0x00000fff & 0xa0caffee: cmparg is sign-extended 0xfee = -18

    That means the op tries to do this:

    (futex |= (1 << (-849))) == -18

    which is completely bogus. The new check of op in the code is:

    if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28)) {
    if (oparg < 0 || oparg > 31)
    return -EINVAL;
    oparg = 1 << oparg;
    }

    which results obviously in the "Invalid argument" errno:

    FAIL: futex
    ===========

    futex(0x7fabd78bcffc, 0x5, 0xfacefeed, 0xb, 0x7fabd78bcffc, 0xa0caffee) = -1: Invalid argument
    futex.test: failed test: ../futex failed with code 1

    So let us soften the failure to print only a (ratelimited) message, crop
    the value and continue as if it were right. When userspace keeps up, we
    can switch this to return -EINVAL again.

    [v2] Do not return 0 immediatelly, proceed with the cropped value.

    Fixes: 30d6e0a4190d ("futex: Remove duplicated code and fix undefined behaviour")
    Signed-off-by: Jiri Slaby <jslaby@suse.cz>
    Cc: Ingo Molnar <mingo@redhat.com>
    Cc: Peter Zijlstra <peterz@infradead.org>
    Cc: Darren Hart <dvhart@infradead.org>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
    Cc: Guenter Roeck <linux@roeck-us.net>
    Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

    ---
    kernel/futex.c | 12 ++++++++++--
    1 file changed, 10 insertions(+), 2 deletions(-)

    --- a/kernel/futex.c
    +++ b/kernel/futex.c
    @@ -1479,8 +1479,16 @@ static int futex_atomic_op_inuser(unsign
    int oldval, ret;

    if (encoded_op & (FUTEX_OP_OPARG_SHIFT << 28)) {
    - if (oparg < 0 || oparg > 31)
    - return -EINVAL;
    + if (oparg < 0 || oparg > 31) {
    + char comm[sizeof(current->comm)];
    + /*
    + * kill this print and return -EINVAL when userspace
    + * is sane again
    + */
    + pr_info_ratelimited("futex_wake_op: %s tries to shift op by %d; fix this program\n",
    + get_task_comm(comm, current), oparg);
    + oparg &= 31;
    + }
    oparg = 1 << oparg;
    }


    \
     
     \ /
      Last update: 2020-04-22 12:00    [W:2.532 / U:0.440 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site