Tested with gcc 14.2 and the Linux kernel compiling for amd64. This is
at Linux next-20241127. This was already the case on gcc 13 (no idea
about earlier versions), I tested 14 to see if the problem is gone.

In the particular case I ran into a prediction concerning the return
value of __access_ok is not correctly acted upon.

#define access_ok(addr, size) likely(__access_ok(addr, size))

then in __access_ok:
return valid_user_address(sum) && sum >= (__force unsigned long)ptr;

The expected asm contains 2 branches both with a jump towards the end
of the function in the failing case. Instead jumps are emitted if
everything works out.

Interestingly the issue fixes itself if I split the if statement like so:
-               return valid_user_address(sum) && sum >= (__force
unsigned long)ptr;
+               if (!valid_user_address(sum))
+                       return false;
+               return sum >= (__force unsigned long)ptr;

Routine at hand (trimmed):
  static inline __must_check unsigned long
  _inline_copy_to_user(void __user *to, const void *from, unsigned long n)
  {
         if (access_ok(to, n)) {
                 instrument_copy_to_user(to, from, n);
                 n = raw_copy_to_user(to, from, n);
         }
         return n;
  }

Bad asm:
Dump of assembler code for function _copy_to_user:
   0xffffffff819713c0 <+0>:     endbr64
   0xffffffff819713c4 <+4>:     mov    %rdx,%rcx
   0xffffffff819713c7 <+7>:     mov    %rdx,%rax
   0xffffffff819713ca <+10>:    xor    %edx,%edx
   0xffffffff819713cc <+12>:    add    %rdi,%rcx
   0xffffffff819713cf <+15>:    setb   %dl
   0xffffffff819713d2 <+18>:    movabs $0x123456789abcdef,%r8
   0xffffffff819713dc <+28>:    test   %rdx,%rdx
   0xffffffff819713df <+31>:    jne    0xffffffff819713e6 <_copy_to_user+38>

this jumps to the exit

   0xffffffff819713e1 <+33>:    cmp    %rcx,%r8
   0xffffffff819713e4 <+36>:    jae    0xffffffff819713eb <_copy_to_user+43>

this jumps over exiting the func to do the actual work, even though
doing the compiler is hinted this is the expected state

   0xffffffff819713e6 <+38>:    jmp    0xffffffff8206b160 <__x86_return_thunk>
   0xffffffff819713eb <+43>:    nop
   0xffffffff819713ec <+44>:    nop
   0xffffffff819713ed <+45>:    nop
   0xffffffff819713ee <+46>:    mov    %rax,%rcx
   0xffffffff819713f1 <+49>:    rep movsb %ds:(%rsi),%es:(%rdi)
   0xffffffff819713f3 <+51>:    nop
   0xffffffff819713f4 <+52>:    nop
   0xffffffff819713f5 <+53>:    nop
   0xffffffff819713f6 <+54>:    mov    %rcx,%rax
   0xffffffff819713f9 <+57>:    nop
   0xffffffff819713fa <+58>:    nop
   0xffffffff819713fb <+59>:    nop
   0xffffffff819713fc <+60>:    jmp    0xffffffff8206b160 <__x86_return_thunk>

Good asm:
Dump of assembler code for function _copy_to_user:
   0xffffffff819713f0 <+0>:     endbr64
   0xffffffff819713f4 <+4>:     xor    %r8d,%r8d
   0xffffffff819713f7 <+7>:     mov    %rdx,%rax
   0xffffffff819713fa <+10>:    add    %rdi,%rdx
   0xffffffff819713fd <+13>:    setb   %r8b
   0xffffffff81971401 <+17>:    movabs $0x123456789abcdef,%rcx
   0xffffffff8197140b <+27>:    cmp    %rdx,%rcx
   0xffffffff8197140e <+30>:    jb     0xffffffff81971426 <_copy_to_user+54>

this only jumps if the condition failed (where the prediction is that it wont)

   0xffffffff81971410 <+32>:    test   %r8,%r8
   0xffffffff81971413 <+35>:    jne    0xffffffff81971426 <_copy_to_user+54>

ditto

   0xffffffff81971415 <+37>:    nop
   0xffffffff81971416 <+38>:    nop
   0xffffffff81971417 <+39>:    nop
   0xffffffff81971418 <+40>:    mov    %rax,%rcx
   0xffffffff8197141b <+43>:    rep movsb %ds:(%rsi),%es:(%rdi)
   0xffffffff8197141d <+45>:    nop
   0xffffffff8197141e <+46>:    nop
   0xffffffff8197141f <+47>:    nop
   0xffffffff81971420 <+48>:    mov    %rcx,%rax
   0xffffffff81971423 <+51>:    nop
   0xffffffff81971424 <+52>:    nop
   0xffffffff81971425 <+53>:    nop
   0xffffffff81971426 <+54>:    jmp    0xffffffff8206b1a0 <__x86_return_thunk>

-- 
Mateusz Guzik <mjguzik gmail.com>

Reply via email to