On 5/13/20 9:09 AM, Christoph Hellwig wrote:
> On Wed, May 13, 2020 at 08:41:57AM -0700, Eric Dumazet wrote:
>>> + * recv* side when msg_control_is_user is set, msg_control is the kernel
>>> + * buffer used for all other cases.
>>> + */
>>> + union {
>>> + void *msg_control;
>>> + void __user *msg_control_user;
>>> + };
>>> + bool msg_control_is_user : 1;
>>
>> Adding a field in this structure seems dangerous.
>>
>> Some users of 'struct msghdr ' define their own struct on the stack,
>> and are unaware of this new mandatory field.
>>
>> This bit contains garbage, crashes are likely to happen ?
>>
>> Look at IPV6_2292PKTOPTIONS for example.
>
> I though of that, an that is why the field is structured as-is. The idea
> is that the field only matters if:
>
> (1) we are in the recvmsg and friends path, and
> (2) msg_control is non-zero
>
> I went through the places that initialize msg_control to find any spot
> that would need an annotation. The IPV6_2292PKTOPTIONS sockopt doesn't
> need one as it is using the msghdr in sendmsg-like context.
>
> That being said while I did the audit I'd appreciate another look from
> people that know the networking code better than me of course.
>
Please try the following syzbot repro, since it crashes after your patch.
// autogenerated by syzkaller (https://github.com/google/syzkaller)
#define _GNU_SOURCE
#include <endian.h>
#include <stdint.h>
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <sys/syscall.h>
#include <sys/types.h>
#include <unistd.h>
uint64_t r[1] = {0xffffffffffffffff};
int main(void)
{
syscall(__NR_mmap, 0x1ffff000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
syscall(__NR_mmap, 0x20000000ul, 0x1000000ul, 7ul, 0x32ul, -1, 0ul);
syscall(__NR_mmap, 0x21000000ul, 0x1000ul, 0ul, 0x32ul, -1, 0ul);
intptr_t res = 0;
// socket(AF_INET6, SOCK_STREAM, IPPROTO_IP) = 3
res = syscall(__NR_socket, 0xaul, 1ul, 0);
if (res != -1)
r[0] = res;
*(uint32_t*)0x20000080 = 7;
// setsockopt(3, SOL_IPV6, IPV6_2292HOPLIMIT, [7], 4) = 0
syscall(__NR_setsockopt, r[0], 0x29, 8, 0x20000080ul, 4ul);
*(uint32_t*)0x20000040 = 0x18ff8;
// getsockopt(3, SOL_IPV6, IPV6_2292PKTOPTIONS,
"\24\0\0\0\0\0\0\0)\0\0\0\10\0\0\0\1\0\0\0\0\0\0\0", [102392->24]) = 0
syscall(__NR_getsockopt, r[0], 0x29, 6, 0x20004040ul, 0x20000040ul);
return 0;
}
