| 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
| ||
|
Message-ID: <aMlp_Covl57nnVoe@J2N7QTR9R3.cambridge.arm.com>
Date: Tue, 16 Sep 2025 14:45:32 +0100
From: Mark Rutland <mark.rutland@....com>
To: Yeoreum Yun <yeoreum.yun@....com>
Cc: Catalin Marinas <catalin.marinas@....com>,
Will Deacon <will@...nel.org>, broonie@...nel.org, maz@...nel.org,
oliver.upton@...ux.dev, joey.gouly@....com, james.morse@....com,
ardb@...nel.org, scott@...amperecomputing.com,
suzuki.poulose@....com, yuzenghui@...wei.com,
linux-arm-kernel@...ts.infradead.org, kvmarm@...ts.linux.dev,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH RESEND v7 4/6] arm64: futex: refactor futex atomic
operation
On Tue, Sep 16, 2025 at 02:27:37PM +0100, Yeoreum Yun wrote:
> Hi Mark,
>
> [...]
> > > diff --git a/arch/arm64/include/asm/futex.h b/arch/arm64/include/asm/futex.h
> > > index 1d6d9f856ac5..0aeda7ced2c0 100644
> > > --- a/arch/arm64/include/asm/futex.h
> > > +++ b/arch/arm64/include/asm/futex.h
> > > @@ -126,6 +126,60 @@ LSUI_FUTEX_ATOMIC_OP(or, ldtset, al)
> > > LSUI_FUTEX_ATOMIC_OP(andnot, ldtclr, al)
> > > LSUI_FUTEX_ATOMIC_OP(set, swpt, al)
> > >
> > > +
> > > +#define LSUI_CMPXCHG_HELPER(suffix, start_bit) \
> > > +static __always_inline int \
> > > +__lsui_cmpxchg_helper_##suffix(u64 __user *uaddr, u32 oldval, u32 newval) \
> > > +{ \
> > > + int ret = 0; \
> > > + u64 oval, nval, tmp; \
> > > + \
> > > + asm volatile("//__lsui_cmpxchg_helper_" #suffix "\n" \
> > > + __LSUI_PREAMBLE \
> > > +" prfm pstl1strm, %2\n" \
> > > +"1: ldtr %x1, %2\n" \
> > > +" mov %x3, %x1\n" \
> > > +" bfi %x1, %x5, #" #start_bit ", #32\n" \
> > > +" bfi %x3, %x6, #" #start_bit ", #32\n" \
> > > +" mov %x4, %x1\n" \
> > > +"2: caslt %x1, %x3, %2\n" \
> > > +" sub %x1, %x1, %x4\n" \
> > > +" cbz %x1, 3f\n" \
> > > +" mov %w0, %w7\n" \
> > > +"3:\n" \
> > > +" dmb ish\n" \
> > > +"4:\n" \
> > > + _ASM_EXTABLE_UACCESS_ERR(1b, 4b, %w0) \
> > > + _ASM_EXTABLE_UACCESS_ERR(2b, 4b, %w0) \
> > > + : "+r" (ret), "=&r" (oval), "+Q" (*uaddr), "=&r" (nval), "=&r" (tmp) \
> > > + : "r" (oldval), "r" (newval), "Ir" (-EAGAIN) \
> > > + : "memory"); \
> > > + \
> > > + return ret; \
> > > +}
> > > +
> > > +LSUI_CMPXCHG_HELPER(lo, 0)
> > > +LSUI_CMPXCHG_HELPER(hi, 32)
> > > +
> > > +static __always_inline int
> > > +__lsui_cmpxchg_helper(u32 __user *uaddr, u32 oldval, u32 newval, u32 *oval)
> > > +{
> > > + int ret;
> > > + unsigned long uaddr_al;
> > > +
> > > + uaddr_al = ALIGN_DOWN((unsigned long)uaddr, sizeof(u64));
> > > +
> > > + if (uaddr_al != (unsigned long)uaddr)
> > > + ret = __lsui_cmpxchg_helper_hi((u64 __user *)uaddr_al, oldval, newval);
> > > + else
> > > + ret = __lsui_cmpxchg_helper_lo((u64 __user *)uaddr_al, oldval, newval);
> > > +
> > > + if (!ret)
> > > + *oval = oldval;
> > > +
> > > + return ret;
> > > +}
> >
> > I think Will expects that you do more of this in C, e.g. have a basic
> > user cmpxchg on a 64-bit type, e.g.
> >
> > /*
> > * NOTE: *oldp is NOT updated if a fault is taken.
> > */
> > static __always_inline int
> > user_cmpxchg64_release(u64 __usr *addr, u64 *oldp, u64 new)
> > {
> > int err = 0;
> >
> > asm volatile(
> > __LSUI_PREAMBLE
> > "1: caslt %x[old], %x[new], %[addr]\n"
> > "2:\n"
> > _ASM_EXTABLE_UACCESS_ERR(1b, 4b, %w0)
> > : [addr] "+Q" (addr),
> > [old] "+r" (*oldp)
> > : [new] "r" (new)
> > : "memory"
> > );
> >
> > return err;
> > }
> >
> > That should be the *only* assembly you need to implement.
> >
> > Atop that, have a wrapper that uses get_user() and that helper above to
> > implement the 32-bit user cmpxchg, with all the bit manipulation in C:
>
> Thanks for your suggestion. But small question.
> I think it's enough to use usafe_get_user() instead of get_user() in here
> since when FEAT_LSUI enabled, it doeesn't need to call
> uaccess_ttbr0_enable()/disable().
Regardless of uaccess_ttbr0_enable() and uaccess_ttbr0_disable()
specifically, API-wise unsafe_get_user() is only supposed to be called
between user_access_begin() and user_access_end(), and there's some
stuff we probably want to add there (e.g. might_fault(), which
unsafe_get_user() lacks today).
Do we call those?
Mark.
Powered by blists - more mailing lists