2023-11-04 09:26:06

by Nadav Amit

[permalink] [raw]
Subject: Missing clobber on alternative use on Linux UM 32-bit

I was reading (again) the x86 C macro of “alternative()” and I was a bit
surprised it does clobber the flags (“cc”) as a precaution.

#define alternative(oldinstr, newinstr, ft_flags) \
asm_inline volatile (ALTERNATIVE(oldinstr, newinstr, ft_flags) : : : "memory")

Actually there seems to be only one instance of problematic cases - in um/32-bit:

#define mb() alternative("lock; addl $0,0(%%esp)", "mfence", X86_FEATURE_XMM2)
#define rmb() alternative("lock; addl $0,0(%%esp)", "lfence", X86_FEATURE_XMM2)
#define wmb() alternative("lock; addl $0,0(%%esp)", "sfence", X86_FEATURE_XMM)

Presumably, if XMM or XMM2 are not supported, there would be instances where addl
would be able to change eflags arithmetic flags without the compiler being aware
of it.

As it only affects 32-bit Linux UM - I don’t easily have an environment to test
the fix. An alternative (word-pun unintended) is to add “cc” as a precaution
to the alternative macro.


2023-11-04 09:35:10

by Anton Ivanov

[permalink] [raw]
Subject: Re: Missing clobber on alternative use on Linux UM 32-bit

On 04/11/2023 09:25, Nadav Amit wrote:
> I was reading (again) the x86 C macro of “alternative()” and I was a bit
> surprised it does clobber the flags (“cc”) as a precaution.
>
> #define alternative(oldinstr, newinstr, ft_flags) \
> asm_inline volatile (ALTERNATIVE(oldinstr, newinstr, ft_flags) : : : "memory")
>
> Actually there seems to be only one instance of problematic cases - in um/32-bit:
>
> #define mb() alternative("lock; addl $0,0(%%esp)", "mfence", X86_FEATURE_XMM2)
> #define rmb() alternative("lock; addl $0,0(%%esp)", "lfence", X86_FEATURE_XMM2)
> #define wmb() alternative("lock; addl $0,0(%%esp)", "sfence", X86_FEATURE_XMM)
>
> Presumably, if XMM or XMM2 are not supported, there would be instances where addl
> would be able to change eflags arithmetic flags without the compiler being aware
> of it.
>
> As it only affects 32-bit Linux UM - I don’t easily have an environment to test
> the fix. An alternative (word-pun unintended) is to add “cc” as a precaution
> to the alternative macro.
>
>
> _______________________________________________
> linux-um mailing list
> [email protected]
> http://lists.infradead.org/mailman/listinfo/linux-um

Application alternatives in um is presently a NOP. It always uses the
"blunt and heavy instrument" - the most conservative option.

It is on the TODO list.

--
Anton R. Ivanov
https://www.kot-begemot.co.uk/

2023-11-04 09:41:33

by Nadav Amit

[permalink] [raw]
Subject: Re: Missing clobber on alternative use on Linux UM 32-bit



> On Nov 4, 2023, at 11:34 AM, Anton Ivanov <[email protected]> wrote:
>
> On 04/11/2023 09:25, Nadav Amit wrote:
>>
>> I was reading (again) the x86 C macro of “alternative()” and I was a bit
>> surprised it does clobber the flags (“cc”) as a precaution.
>>
>> #define alternative(oldinstr, newinstr, ft_flags) \
>> asm_inline volatile (ALTERNATIVE(oldinstr, newinstr, ft_flags) : : : "memory")
>>
>> Actually there seems to be only one instance of problematic cases - in um/32-bit:
>>
>> #define mb() alternative("lock; addl $0,0(%%esp)", "mfence", X86_FEATURE_XMM2)
>> #define rmb() alternative("lock; addl $0,0(%%esp)", "lfence", X86_FEATURE_XMM2)
>> #define wmb() alternative("lock; addl $0,0(%%esp)", "sfence", X86_FEATURE_XMM)
>>
>> Presumably, if XMM or XMM2 are not supported, there would be instances where addl
>> would be able to change eflags arithmetic flags without the compiler being aware
>> of it.
>>
>> As it only affects 32-bit Linux UM - I don’t easily have an environment to test
>> the fix. An alternative (word-pun unintended) is to add “cc” as a precaution
>> to the alternative macro.
>>
> Application alternatives in um is presently a NOP. It always uses the "blunt and heavy instrument" - the most conservative option.
>
> It is on the TODO list.

Thanks for the quick response. But I don’t see how it prevents the problem
(it actually makes it worse - affecting XMM/XMM2 CPUs as well) since you
keep the “lock; addl $0,0(%%esp)” in the running code, affecting eflags
without telling the compiler that you do so through a “cc” clobber.

2023-11-05 15:47:44

by David Laight

[permalink] [raw]
Subject: RE: Missing clobber on alternative use on Linux UM 32-bit

From: Nadav Amit
> Sent: 04 November 2023 09:41
>
> > On Nov 4, 2023, at 11:34 AM, Anton Ivanov <[email protected]> wrote:
> >
> > On 04/11/2023 09:25, Nadav Amit wrote:
> >>
> >> I was reading (again) the x86 C macro of “alternative()” and I was a bit
> >> surprised it does clobber the flags (“cc”) as a precaution.
> >>
> >> #define alternative(oldinstr, newinstr, ft_flags) \
> >> asm_inline volatile (ALTERNATIVE(oldinstr, newinstr, ft_flags) : : : "memory")
> >>
> >> Actually there seems to be only one instance of problematic cases - in um/32-bit:
> >>
> >> #define mb() alternative("lock; addl $0,0(%%esp)", "mfence", X86_FEATURE_XMM2)
> >> #define rmb() alternative("lock; addl $0,0(%%esp)", "lfence", X86_FEATURE_XMM2)
> >> #define wmb() alternative("lock; addl $0,0(%%esp)", "sfence", X86_FEATURE_XMM)
> >>
> >> Presumably, if XMM or XMM2 are not supported, there would be instances where addl
> >> would be able to change eflags arithmetic flags without the compiler being aware
> >> of it.
> >>
> >> As it only affects 32-bit Linux UM - I don’t easily have an environment to test
> >> the fix. An alternative (word-pun unintended) is to add “cc” as a precaution
> >> to the alternative macro.
> >>
> > Application alternatives in um is presently a NOP. It always uses the "blunt and heavy instrument" -
> the most conservative option.
> >
> > It is on the TODO list.
>
> Thanks for the quick response. But I don’t see how it prevents the problem
> (it actually makes it worse - affecting XMM/XMM2 CPUs as well) since you
> keep the “lock; addl $0,0(%%esp)” in the running code, affecting eflags
> without telling the compiler that you do so through a “cc” clobber.

gcc always assumes that inline asm changes "cc" - there is no need
to add a 'clobber' for it.

David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)

2023-11-06 11:33:37

by Nadav Amit

[permalink] [raw]
Subject: Re: Missing clobber on alternative use on Linux UM 32-bit



> On Nov 5, 2023, at 5:47 PM, David Laight <[email protected]> wrote:
>
> gcc always assumes that inline asm changes "cc" - there is no need
> to add a 'clobber' for it.

Thanks. I was unaware of this behavior.