[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v7 2/5] x86/asm: add volatile, clobbers and zero-length check in inline memcmp



hpa in To:.

On Tue, Jul 21, 2026 at 12:56:43PM -0300, Mauricio Faria de Oliveira wrote:
> Add the volatile qualifier and clobbers parameter to prevent bugs with
> instruction reordering and optimization.
> 
> Also check the zero-length case, as the 'repe' prefix does not run the
> 'cmpsb' instruction if the 'count' register is zero, which doesn't set

Please use capital letters for insns: REPE, CMPSB and you don't need to put
words in '' - it reads fine without them.

> the condition-code/zero flag, so the result is based on a stale flag.
> 
> Those are pre-existing issues found by Sashiko.
> 
> Link: 
> https://sashiko.dev/#/patchset/20260701-pvh-kasan-inline-v6-0-ba99045dfa9f%40igalia.com
> Signed-off-by: Mauricio Faria de Oliveira <mfo@xxxxxxxxxx>
> ---
>  arch/x86/include/asm/shared/string.h | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/x86/include/asm/shared/string.h 
> b/arch/x86/include/asm/shared/string.h
> index 
> 02b92927553f7b8e1c87e6122bbaa70439e57ea7..166274e44f3cb49e3dccab3cdac281d67aef5d44
>  100644
> --- a/arch/x86/include/asm/shared/string.h
> +++ b/arch/x86/include/asm/shared/string.h
> @@ -11,8 +11,12 @@ static __always_inline int __inline_memcmp(const void *s1, 
> const void *s2, size_
>  {
>       bool diff;
>  
> -     asm("repe cmpsb"
> -         : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len));
> +     if (len == 0)
> +             return 0;
> +
> +     asm volatile("repe cmpsb"
> +                  : "=@ccnz" (diff), "+D" (s1), "+S" (s2), "+c" (len)
> +                  : : "cc", "memory");
>  
>       return diff;
>  }

So this is a fix which should probably go to stable, I think. Going back into
git history, it points to

  62bd0337d0c4 ("Top header file for new x86 setup code")

from 2007. And we have carried it this way through the years and who knows
what hit this or not. So please make this the first patch in your set - you
can even send it separately so that I can get route it through stable.

Then you can base the rest ontop.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.