[PATCH] D56571: [RFC prototype] Implementation of asm-goto support in clang

Kees Cook via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 6 05:55:08 PST 2019


kees added a comment.

Not sure if this is the fault of the LLVM half or the Clang half, but I'm seeing mis-compilations in the current patches (llvm ca1e713fdd4fab5273b36ba6f292a844fca4cb2d with D53765 <https://reviews.llvm.org/D53765>.185490 and clang 01879634f01bdbfac4636ebe03b68e85b20cd664 with D56571 <https://reviews.llvm.org/D56571>.185489). My earlier builds were okay (llvm b1650507d25d28a03f30626843b7b133796597b4 with D53765 <https://reviews.llvm.org/D53765>.183738 and clang 61738985ebe78eeff6cfae7f97543d3456bac25a with D56571 <https://reviews.llvm.org/D56571>.181973).

I narrowed the failure down to the kernel's move_addr_to_user() function:

  static int move_addr_to_user(struct sockaddr_storage *kaddr, int klen,
                               void __user *uaddr, int __user *ulen)
  {
          int err;
          int len;
  
          BUG_ON(klen > sizeof(struct sockaddr_storage));
          err = get_user(len, ulen);
          if (err)
                  return err;
          if (len > klen)
                  len = klen;
          if (len < 0)
                  return -EINVAL;
          if (len) {
                  if (audit_sockaddr(klen, kaddr))
                          return -ENOMEM;
                  if (copy_to_user(uaddr, kaddr, len))
                          return -EFAULT;
          }
          return __put_user(klen, ulen);
  }

Working build produces:

  ffffffff81c217a0 <move_addr_to_user>:
  ffffffff81c217a0:       e8 5b fc 3d 00          callq  ffffffff82001400 <__fentry__>
  ffffffff81c217a5:       81 fe 81 00 00 00       cmp    $0x81,%esi
  ffffffff81c217ab:       0f 83 c8 00 00 00       jae    ffffffff81c21879 <move_addr_to_user+0xd9>
  ffffffff81c217b1:       55                      push   %rbp
  ffffffff81c217b2:       48 89 e5                mov    %rsp,%rbp
  ffffffff81c217b5:       41 57                   push   %r15
  ffffffff81c217b7:       41 56                   push   %r14
  ffffffff81c217b9:       41 55                   push   %r13
  ffffffff81c217bb:       41 54                   push   %r12
  ffffffff81c217bd:       53                      push   %rbx
  ffffffff81c217be:       49 89 ce                mov    %rcx,%r14
  ffffffff81c217c1:       49 89 d7                mov    %rdx,%r15
  ffffffff81c217c4:       41 89 f5                mov    %esi,%r13d
  ffffffff81c217c7:       49 89 fc                mov    %rdi,%r12
  ffffffff81c217ca:       48 c7 c7 36 35 88 82    mov    $0xffffffff82883536,%rdi
  ffffffff81c217d1:       be da 00 00 00          mov    $0xda,%esi
  ffffffff81c217d6:       e8 25 8b 5c ff          callq  ffffffff811ea300 <__might_fault>
  ffffffff81c217db:       4c 89 f0                mov    %r14,%rax
  ffffffff81c217de:       e8 2d f6 2f 00          callq  ffffffff81f20e10 <__get_user_4>
  ffffffff81c217e3:       85 c0                   test   %eax,%eax
  ffffffff81c217e5:       75 68                   jne    ffffffff81c2184f <move_addr_to_user+0xaf>
  ffffffff81c217e7:       48 89 d3                mov    %rdx,%rbx
  ffffffff81c217ea:       44 39 eb                cmp    %r13d,%ebx
  ffffffff81c217ed:       41 0f 4f dd             cmovg  %r13d,%ebx
  ffffffff81c217f1:       85 db                   test   %ebx,%ebx
  ffffffff81c217f3:       78 65                   js     ffffffff81c2185a <move_addr_to_user+0xba>
  ffffffff81c217f5:       74 48                   je     ffffffff81c2183f <move_addr_to_user+0x9f>
  ffffffff81c217f7:       65 48 8b 04 25 00 4e    mov    %gs:0x14e00,%rax
  ffffffff81c217fe:       01 00 
  ffffffff81c21800:       48 8b 80 38 07 00 00    mov    0x738(%rax),%rax
  ffffffff81c21807:       48 85 c0                test   %rax,%rax
  ffffffff81c2180a:       74 05                   je     ffffffff81c21811 <move_addr_to_user+0x71>
  ffffffff81c2180c:       83 38 00                cmpl   $0x0,(%rax)
  ffffffff81c2180f:       74 50                   je     ffffffff81c21861 <move_addr_to_user+0xc1>
  ffffffff81c21811:       48 63 db                movslq %ebx,%rbx
  ffffffff81c21814:       4c 89 e7                mov    %r12,%rdi
  ffffffff81c21817:       48 89 de                mov    %rbx,%rsi
  ffffffff81c2181a:       ba 01 00 00 00          mov    $0x1,%edx
  ffffffff81c2181f:       e8 3c 75 5f ff          callq  ffffffff81218d60 <__check_object_size>
  ffffffff81c21824:       4c 89 ff                mov    %r15,%rdi
  ffffffff81c21827:       4c 89 e6                mov    %r12,%rsi
  ffffffff81c2182a:       48 89 da                mov    %rbx,%rdx
  ffffffff81c2182d:       e8 ae 84 99 ff          callq  ffffffff815b9ce0 <_copy_to_user>
  ffffffff81c21832:       48 89 c1                mov    %rax,%rcx
  ffffffff81c21835:       b8 f2 ff ff ff          mov    $0xfffffff2,%eax
  ffffffff81c2183a:       48 85 c9                test   %rcx,%rcx
  ffffffff81c2183d:       75 10                   jne    ffffffff81c2184f <move_addr_to_user+0xaf>
  ffffffff81c2183f:       90                      nop
  ffffffff81c21840:       90                      nop
  ffffffff81c21841:       90                      nop
  ffffffff81c21842:       b8 f2 ff ff ff          mov    $0xfffffff2,%eax
  ffffffff81c21847:       45 89 2e                mov    %r13d,(%r14)
  ffffffff81c2184a:       31 c0                   xor    %eax,%eax
  ffffffff81c2184c:       90                      nop
  ffffffff81c2184d:       90                      nop
  ffffffff81c2184e:       90                      nop
  ffffffff81c2184f:       5b                      pop    %rbx
  ffffffff81c21850:       41 5c                   pop    %r12
  ffffffff81c21852:       41 5d                   pop    %r13
  ffffffff81c21854:       41 5e                   pop    %r14
  ffffffff81c21856:       41 5f                   pop    %r15
  ffffffff81c21858:       5d                      pop    %rbp
  ffffffff81c21859:       c3                      retq   
  ffffffff81c2185a:       b8 ea ff ff ff          mov    $0xffffffea,%eax
  ffffffff81c2185f:       eb ee                   jmp    ffffffff81c2184f <move_addr_to_user+0xaf>
  ffffffff81c21861:       44 89 ef                mov    %r13d,%edi
  ffffffff81c21864:       4c 89 e6                mov    %r12,%rsi
  ffffffff81c21867:       e8 f4 1c 52 ff          callq  ffffffff81143560 <__audit_sockaddr>
  ffffffff81c2186c:       89 c1                   mov    %eax,%ecx
  ffffffff81c2186e:       b8 f4 ff ff ff          mov    $0xfffffff4,%eax
  ffffffff81c21873:       85 c9                   test   %ecx,%ecx
  ffffffff81c21875:       75 d8                   jne    ffffffff81c2184f <move_addr_to_user+0xaf>
  ffffffff81c21877:       eb 98                   jmp    ffffffff81c21811 <move_addr_to_user+0x71>
  ffffffff81c21879:       0f 0b                   ud2    
  ffffffff81c2187b:       0f 1f 44 00 00          nopl   0x0(%rax,%rax,1)

The bad compilation produces:

  ffffffff81c21700 <move_addr_to_user>:
  ffffffff81c21700:       e8 fb fc 3d 00          callq  ffffffff82001400 <__fentry__>
  ffffffff81c21705:       81 fe 81 00 00 00       cmp    $0x81,%esi
  ffffffff81c2170b:       0f 83 cc 00 00 00       jae    ffffffff81c217dd <move_addr_to_user+0xdd>
  ffffffff81c21711:       55                      push   %rbp
  ffffffff81c21712:       48 89 e5                mov    %rsp,%rbp
  ffffffff81c21715:       41 57                   push   %r15
  ffffffff81c21717:       41 56                   push   %r14
  ffffffff81c21719:       41 55                   push   %r13
  ffffffff81c2171b:       41 54                   push   %r12
  ffffffff81c2171d:       53                      push   %rbx
  ffffffff81c2171e:       49 89 ce                mov    %rcx,%r14
  ffffffff81c21721:       49 89 d7                mov    %rdx,%r15
  ffffffff81c21724:       41 89 f5                mov    %esi,%r13d
  ffffffff81c21727:       49 89 fc                mov    %rdi,%r12
  ffffffff81c2172a:       48 c7 c7 36 35 88 82    mov    $0xffffffff82883536,%rdi
  ffffffff81c21731:       be da 00 00 00          mov    $0xda,%esi
  ffffffff81c21736:       e8 a5 8b 5c ff          callq  ffffffff811ea2e0 <__might_fault>
  ffffffff81c2173b:       4c 89 f0                mov    %r14,%rax
  ffffffff81c2173e:       e8 bd f5 2f 00          callq  ffffffff81f20d00 <__get_user_4>
  ffffffff81c21743:       85 c0                   test   %eax,%eax
  ffffffff81c21745:       0f 85 87 00 00 00       jne    ffffffff81c217d2 <move_addr_to_user+0xd2>
  ffffffff81c2174b:       48 89 d3                mov    %rdx,%rbx
  ffffffff81c2174e:       44 39 eb                cmp    %r13d,%ebx
  ffffffff81c21751:       41 0f 4f dd             cmovg  %r13d,%ebx
  ffffffff81c21755:       85 db                   test   %ebx,%ebx
  ffffffff81c21757:       78 57                   js     ffffffff81c217b0 <move_addr_to_user+0xb0>
  ffffffff81c21759:       74 48                   je     ffffffff81c217a3 <move_addr_to_user+0xa3>
  ffffffff81c2175b:       65 48 8b 04 25 00 4e    mov    %gs:0x14e00,%rax
  ffffffff81c21762:       01 00 
  ffffffff81c21764:       48 8b 80 38 07 00 00    mov    0x738(%rax),%rax
  ffffffff81c2176b:       48 85 c0                test   %rax,%rax
  ffffffff81c2176e:       74 05                   je     ffffffff81c21775 <move_addr_to_user+0x75>
  ffffffff81c21770:       83 38 00                cmpl   $0x0,(%rax)
  ffffffff81c21773:       74 42                   je     ffffffff81c217b7 <move_addr_to_user+0xb7>
  ffffffff81c21775:       48 63 db                movslq %ebx,%rbx
  ffffffff81c21778:       4c 89 e7                mov    %r12,%rdi
  ffffffff81c2177b:       48 89 de                mov    %rbx,%rsi
  ffffffff81c2177e:       ba 01 00 00 00          mov    $0x1,%edx
  ffffffff81c21783:       e8 b8 75 5f ff          callq  ffffffff81218d40 <__check_object_size>
  ffffffff81c21788:       4c 89 ff                mov    %r15,%rdi
  ffffffff81c2178b:       4c 89 e6                mov    %r12,%rsi
  ffffffff81c2178e:       48 89 da                mov    %rbx,%rdx
  ffffffff81c21791:       e8 4a 85 99 ff          callq  ffffffff815b9ce0 <_copy_to_user>
  ffffffff81c21796:       48 89 c1                mov    %rax,%rcx
  ffffffff81c21799:       b8 f2 ff ff ff          mov    $0xfffffff2,%eax
  ffffffff81c2179e:       48 85 c9                test   %rcx,%rcx
  ffffffff81c217a1:       75 2f                   jne    ffffffff81c217d2 <move_addr_to_user+0xd2>
  ffffffff81c217a3:       90                      nop
  ffffffff81c217a4:       90                      nop
  ffffffff81c217a5:       90                      nop
  ffffffff81c217a6:       b8 f2 ff ff ff          mov    $0xfffffff2,%eax
  ffffffff81c217ab:       45 89 2e                mov    %r13d,(%r14)
  ffffffff81c217ae:       31 c0                   xor    %eax,%eax
  ffffffff81c217b0:       b8 ea ff ff ff          mov    $0xffffffea,%eax
  ffffffff81c217b5:       eb 1b                   jmp    ffffffff81c217d2 <move_addr_to_user+0xd2>
  ffffffff81c217b7:       44 89 ef                mov    %r13d,%edi
  ffffffff81c217ba:       4c 89 e6                mov    %r12,%rsi
  ffffffff81c217bd:       e8 9e 1d 52 ff          callq  ffffffff81143560 <__audit_sockaddr>
  ffffffff81c217c2:       89 c1                   mov    %eax,%ecx
  ffffffff81c217c4:       b8 f4 ff ff ff          mov    $0xfffffff4,%eax
  ffffffff81c217c9:       85 c9                   test   %ecx,%ecx
  ffffffff81c217cb:       75 05                   jne    ffffffff81c217d2 <move_addr_to_user+0xd2>
  ffffffff81c217cd:       eb a6                   jmp    ffffffff81c21775 <move_addr_to_user+0x75>
  ffffffff81c217cf:       90                      nop
  ffffffff81c217d0:       90                      nop
  ffffffff81c217d1:       90                      nop
  ffffffff81c217d2:       5b                      pop    %rbx
  ffffffff81c217d3:       41 5c                   pop    %r12
  ffffffff81c217d5:       41 5d                   pop    %r13
  ffffffff81c217d7:       41 5e                   pop    %r14
  ffffffff81c217d9:       41 5f                   pop    %r15
  ffffffff81c217db:       5d                      pop    %rbp
  ffffffff81c217dc:       c3                      retq   
  ffffffff81c217dd:       0f 0b                   ud2    
  ffffffff81c217df:       90                      nop

The bad sequences starts at the je following js (at ffffffff81c217f5 in the good build and at ffffffff81c21759 in bad build). In the good build, the jump leads to:

  ffffffff81c2183f:       90                      nop
  ffffffff81c21840:       90                      nop
  ffffffff81c21841:       90                      nop
  ffffffff81c21842:       b8 f2 ff ff ff          mov    $0xfffffff2,%eax
  ffffffff81c21847:       45 89 2e                mov    %r13d,(%r14)
  ffffffff81c2184a:       31 c0                   xor    %eax,%eax
  ffffffff81c2184c:       90                      nop
  ffffffff81c2184d:       90                      nop
  ffffffff81c2184e:       90                      nop
  ffffffff81c2184f:       5b                      pop    %rbx
  ffffffff81c21850:       41 5c                   pop    %r12
  ...

In the bad build, the mov, mov, xor block does not fall through to function exit, instead it overwrites %eax before jumping to function exit:

  ffffffff81c217a3:       90                      nop
  ffffffff81c217a4:       90                      nop
  ffffffff81c217a5:       90                      nop
  ffffffff81c217a6:       b8 f2 ff ff ff          mov    $0xfffffff2,%eax
  ffffffff81c217ab:       45 89 2e                mov    %r13d,(%r14)
  ffffffff81c217ae:       31 c0                   xor    %eax,%eax
  ffffffff81c217b0:       b8 ea ff ff ff          mov    $0xffffffea,%eax
  ffffffff81c217b5:       eb 1b                   jmp    ffffffff81c217d2 <move_addr_to_user+0xd2>
  ...
  ffffffff81c217d2:       5b                      pop    %rbx
  ffffffff81c217d3:       41 5c                   pop    %r12
  ...

I've not been able to narrow this down further -- most changes to the C code cause the behavior to vanish, so I assume some optimization pass or something is moving things around to avoid the bug.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56571/new/

https://reviews.llvm.org/D56571





More information about the cfe-commits mailing list