lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <aXDfty/c9c9YD8u5@e129823.arm.com>
Date: Wed, 21 Jan 2026 14:16:23 +0000
From: Yeoreum Yun <yeoreum.yun@....com>
To: Will Deacon <will@...nel.org>
Cc: catalin.marinas@....com, maz@...nel.org, broonie@...nel.org,
	oliver.upton@...ux.dev, miko.lenczewski@....com,
	kevin.brodsky@....com, ardb@...nel.org, suzuki.poulose@....com,
	lpieralisi@...nel.org, yangyicong@...ilicon.com,
	scott@...amperecomputing.com, joey.gouly@....com,
	yuzenghui@...wei.com, pbonzini@...hat.com, shuah@...nel.org,
	mark.rutland@....com, arnd@...db.de,
	linux-arm-kernel@...ts.infradead.org, linux-kernel@...r.kernel.org,
	kvmarm@...ts.linux.dev, kvm@...r.kernel.org,
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v11 RESEND 6/9] arm64: futex: support futex with FEAT_LSUI

Hi Will,

> On Mon, Jan 19, 2026 at 10:17:47PM +0000, Yeoreum Yun wrote:
> > > > +"2:\n"
> > > > +	_ASM_EXTABLE_UACCESS_ERR(1b, 2b, %w0)
> > > > +	: "+r" (ret), "+Q" (*uaddr), "+r" (*oldval)
> > > > +	: "r" (newval)
> > > > +	: "memory");
> > >
> > > Don't you need to update *oldval here if the CAS didn't fault?
> >
> > No. if CAS doesn't make fault the oldval update already.
>
> Sorry, it was the "+r" constraint with a pointer dereference that threw
> me but you have the "memory" clobber so it looks like this will work.
>
> > > > +
> > > > +	for (i = 0; i < FUTEX_MAX_LOOPS; i++) {
> > > > +		if (get_user(oval64.raw, uaddr64))
> > > > +			return -EFAULT;
> > >
> > > Since oldval is passed to us as an argument, can we get away with a
> > > 32-bit get_user() here?
> >
> > It's not a probelm. but is there any sigificant difference?
>
> I think the code would be clearer if you only read what you actually
> use.
>
> > > > +		nval64.raw = oval64.raw;
> > > > +
> > > > +		if (futex_on_lo) {
> > > > +			oval64.lo_futex.val = oldval;
> > > > +			nval64.lo_futex.val = newval;
> > > > +		} else {
> > > > +			oval64.hi_futex.val = oldval;
> > > > +			nval64.hi_futex.val = newval;
> > > > +		}
> > > > +
> > > > +		orig64.raw = oval64.raw;
> > > > +
> > > > +		if (__lsui_cmpxchg64(uaddr64, &oval64.raw, nval64.raw))
> > > > +			return -EFAULT;
> > > > +
> > > > +		if (futex_on_lo) {
> > > > +			oldval = oval64.lo_futex.val;
> > > > +			other = oval64.lo_futex.other;
> > > > +			orig_other = orig64.lo_futex.other;
> > > > +		} else {
> > > > +			oldval = oval64.hi_futex.val;
> > > > +			other = oval64.hi_futex.other;
> > > > +			orig_other = orig64.hi_futex.other;
> > > > +		}
> > > > +
> > > > +		if (other == orig_other) {
> > > > +			ret = 0;
> > > > +			break;
> > > > +		}
> > > > +	}
> > > > +
> > > > +	if (!ret)
> > > > +		*oval = oldval;
> > >
> > > Shouldn't we set *oval to the value we got back from the CAS?
> >
> > Since it's a "success" case, the CAS return and oldval must be the same.
> > That's why it doesn't matter to use got back from the CAS.
> > Otherwise, it returns error and *oval doesn't matter for
> > futex_atomic_cmpxchg_inatomic().
>
> Got it, but then the caller you have is very weird because e.g.
> __lsui_futex_atomic_eor() goes and does another get_user() on the next
> iteration instead of using the value returned by the CAS.
>
> It would probably be clearer if you restructured your CAS helper to look
> more like try_cmpxchg() and then the loop around it would be minimal.
> You might need to distinguish the faulting case from the comparison
> failure case with e.g. -EFAULT vs -EAGAIN.


Oh, thanks for pointing this out.I understand your point clearly now.
Yes, I’ll respin the patch accordingly. Thanks again!

--
Sincerely,
Yeoreum Yun

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ