Messages in this thread | | | Date | Sun, 4 Mar 2012 18:01:13 -0600 | From | Jonathan Nieder <> | 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/
| |