lkml.org 
[lkml]   [2012]   [Mar]   [4]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    Subject[regression] Re: [PATCH 2/3] futex: Sanitize cmpxchg_futex_value_locked API
    (reset cc list)
    Hi,

    Michel Lespinasse wrote:

    > This change makes the cmpxchg_futex_value_locked API more similar to the
    > get_futex_value_locked one, returning an error code and updating the
    > original value through a reference argument.
    [...]
    > Acked-by: Tony Luck <tony.luck@intel.com> [ia64]

    Émeric Maschino (cc-ed) is experiencing random crashes, X restarts,
    and so on on Itanium. Bisects to this patch[1].

    Patrick Baggett, investigating, wrote[2]:

    > It doesn't look like the return value (r8) is actually being set beyond
    > initialized to 0. If there is some ia64 instruction that modifies it, GCC
    > doesn't know about it from the inline assembly (r8 doesn't appear in the
    > inputs/outputs list). From looking at the x86 version (agh, inline asm is
    > hard to parse), it does modify the return value based on whether the
    > comparison was a success or not, and the return value is certainly used by
    > the callers.

    And indeed, pinning that variable to that register (why not "prev"
    instead?) looks suspicious.

    Anywhere, here's the potentially problematic patch hunk. Ideas?

    > --- a/arch/ia64/include/asm/futex.h
    > +++ b/arch/ia64/include/asm/futex.h
    > @@ -100,23 +100,26 @@ futex_atomic_op_inuser (int encoded_op, int __user *uaddr)
    > }
    >
    > static inline int
    > -futex_atomic_cmpxchg_inatomic(int __user *uaddr, int oldval, int newval)
    > +futex_atomic_cmpxchg_inatomic(int *uval, int __user *uaddr,
    > + int oldval, int newval)
    > {
    > if (!access_ok(VERIFY_WRITE, uaddr, sizeof(int)))
    > return -EFAULT;
    >
    > {
    > - register unsigned long r8 __asm ("r8");
    > + register unsigned long r8 __asm ("r8") = 0;
    > + unsigned long prev;
    > __asm__ __volatile__(
    > " mf;; \n"
    > " mov ar.ccv=%3;; \n"
    > "[1:] cmpxchg4.acq %0=[%1],%2,ar.ccv \n"
    > " .xdata4 \"__ex_table\", 1b-., 2f-. \n"
    > "[2:]"
    > - : "=r" (r8)
    > + : "=r" (prev)
    > : "r" (uaddr), "r" (newval),
    > "rO" ((long) (unsigned) oldval)
    > : "memory");
    > + *uval = prev;
    > return r8;
    > }
    > }

    Jonathan

    [1] https://bugzilla.kernel.org/show_bug.cgi?id=42757
    [2] http://thread.gmane.org/gmane.linux.debian.ports.ia64/3121/focus=3123
    --
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2012-03-05 01:03    [W:4.250 / U:0.228 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site