Messages in this thread | | | Date | Sun, 27 Mar 2022 23:38:43 +0200 | From | Borislav Petkov <> | Subject | Re: [PATCH v5 1/2] x86/delay: Fix the wrong asm constraint in `delay_loop()` |
| |
On Thu, Mar 10, 2022 at 08:53:05AM +0700, Ammar Faizi wrote: > The asm constraint does not reflect that the asm statement can modify > the value of @loops. But the asm statement in delay_loop() does modify > the @loops. > > Specifiying the wrong constraint may lead to undefined behavior, it may > clobber random stuff (e.g. local variable, important temporary value in > regs, etc.).
This is especially dangerous when the compiler decides to inline the function and since it doesn't know that the value gets modified, it might decide to use it from a register directly without reloading it.
Add that to the commit message pls.
> Fix this by changing the constraint from "a" (as an input) to "+a" (as > an input and output). > > Cc: David Laight <David.Laight@ACULAB.COM> > Cc: Thomas Gleixner <tglx@linutronix.de> > Cc: Ingo Molnar <mingo@redhat.com> > Cc: Borislav Petkov <bp@alien8.de> > Cc: Dave Hansen <dave.hansen@linux.intel.com> > Cc: "H. Peter Anvin" <hpa@zytor.com> > Cc: Jiri Hladky <hladky.jiri@googlemail.com>
All those Ccs in the commit message are not really needed - get_maintainers.pl gives the correct list already.
> Cc: stable@vger.kernel.org # v2.6.27+
I don't see the need for the stable Cc. Or do you have a case where a corruption really does happen?
> Fixes: e01b70ef3eb ("x86: fix bug in arch/i386/lib/delay.c file, delay_loop function")
Commit sha1 (e01b70ef3eb) needs to be at least 12 chars long:
e01b70ef3eb3 ("x86: fix bug in arch/i386/lib/delay.c file, delay_loop function")
This is best fixed by doing:
[core] abbrev = 12
in your .git/config
> Signed-off-by: Ammar Faizi <ammarfaizi2@gnuweeb.org> > --- > arch/x86/lib/delay.c | 4 ++-- > 1 file changed, 2 insertions(+), 2 deletions(-) > > diff --git a/arch/x86/lib/delay.c b/arch/x86/lib/delay.c > index 65d15df6212d..0e65d00e2339 100644 > --- a/arch/x86/lib/delay.c > +++ b/arch/x86/lib/delay.c > @@ -54,8 +54,8 @@ static void delay_loop(u64 __loops) > " jnz 2b \n" > "3: dec %0 \n" > > - : /* we don't need output */ > - :"a" (loops) > + : "+a" (loops) > + : > );
Thx.
-- Regards/Gruss, Boris.
https://people.kernel.org/tglx/notes-about-netiquette
| |