On Mon, 2026-07-20 at 20:26 +0200, Johannes Berg wrote:
>
> +static inline int
> +arch_futex_atomic_op_inuser(int op, u32 oparg, int *oval, u32 __user *uaddr)
> +{
> + u32 oldval, newval;
> + u32 *p = (u32 __force *)uaddr;
> +
> + do {
> + oldval = READ_ONCE(*p);
> +
> + switch (op) {
> + case FUTEX_OP_SET:
> + newval = oparg;
> + break;
> + case FUTEX_OP_ADD:
> + newval = oldval + oparg;
> + break;
> + case FUTEX_OP_OR:
> + newval = oldval | oparg;
> + break;
> + case FUTEX_OP_ANDN:
> + newval = oldval & ~oparg;
> + break;
> + case FUTEX_OP_XOR:
> + newval = oldval ^ oparg;
> + break;
> + default:
> + return -ENOSYS;
> + }
> + } while (!try_cmpxchg(p, &oldval, newval));
> +
> + *oval = oldval;
Wait ... this is also garbage, it should just be the switch() and then
*oval = cmpxchg(p, oldval, newval);
not the loop...
> + /* FIXME - shouldn't it be *uval = cmpxchg()? */
> + *uval = *p;
> + cmpxchg(p, oldval, newval);
And yes, I think the FIXME is right and our uaccess.c implementation is
wrong.
johannes
