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

["H. Peter Anvin" <hpa@xxxxxxxxx>]

On July 22, 2026 10:03:34 AM PDT, Borislav Petkov <bp@xxxxxxxxx> wrote:
>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.
>

I suspect that the zero-length case simply can't happen in the real code, but 
who knows; the use of the code has expanded over the years.