Fw: [Bug 202561] BUG: Null pointer dereference in __skb_unlink()

Fw: [Bug 202561] BUG: Null pointer dereference in __skb_unlink()

[Stephen Hemminger]

https://bugzilla.kernel.org/show_bug.cgi?id=202561


Backtrace:
[    3.589241] BUG: unable to handle kernel NULL pointer dereference at
0000000000000008
[    3.598006] IP: __skb_try_recv_from_queue+0x4e/0x1b0
[    3.606376] Oops: 0002 [#1] PREEMPT SMP NOPTI
[    3.720520] RIP: 0010:__skb_try_recv_from_queue+0x4e/0x1b0
[    3.726645] RSP: 0018:ffffb03400e53ac8 EFLAGS: 00010046
[    3.732470] RAX: ffff9040a78988c8 RBX: ffff904096bbef00 RCX: 000000000000000
[    3.740441] RDX: 0000000000000000 RSI: ffff9040a78988c8 RDI: fff9040a7898800
[    3.748411] RBP: ffffb03400e53ae8 R08: ffffb03400e53bf8 R09: fffb03400e53bfc
[    3.756382] R10: ffff904096989d00 R11: 0000000000000000 R12: fff9040a78988dc
[    3.764345] R13: ffff9040a78988c8 R14: 0000000000000202 R15: fffb03400e53ba8
[    3.772305] FS:  00007feb1ef2f740(0000) GS:ffff9040bfd00000(0000)
knlGS:0000000000000000
[    3.781342] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[    3.787757] CR2: 0000000000000008 CR3: 00000001e87fa000 CR4:
00000000003406a0
[    3.795725] Call Trace:
[    3.798456]  ? preempt_count_add+0x22/0x30
[    3.803027]  __skb_try_recv_datagram+0xe2/0x160
[    3.808086]  __skb_recv_datagram+0x8a/0xc0
[    3.812657]  skb_recv_datagram+0x3a/0x50
[    3.817035]  netlink_recvmsg+0x4e/0x3c0
[    3.821315]  ? copy_msghdr_from_user+0xcf/0x150
[    3.826372]  sock_recvmsg+0x3b/0x50
[    3.830264]  ___sys_recvmsg+0xd4/0x180
[    3.834441]  ? poll_select_copy_remaining+0x140/0x140
[    3.840080]  ? poll_select_copy_remaining+0x140/0x140
[    3.845721]  ? __switch_to_asm+0x34/0x70
[    3.850098]  ? __switch_to_asm+0x40/0x70
[    3.854473]  ? __switch_to_asm+0x34/0x70
[    3.858849]  ? __switch_to_asm+0x40/0x70
[    3.863228]  ? __switch_to_asm+0x34/0x70
[    3.867604]  ? __fget+0x71/0xa0
[    3.871108]  __sys_recvmsg+0x4c/0x90
[    3.875097]  ? __sys_recvmsg+0x4c/0x90
[    3.879280]  SyS_recvmsg+0x9/0x10
[    3.882979]  do_syscall_64+0x7e/0x350
[    3.887064]  ? trace_hardirqs_off_thunk+0x1a/0x1c
[    3.892315]  entry_SYSCALL_64_after_hwframe+0x42/0xb7

Original report from sharathkernel@gmail.com:

NULL POINTER DEFERENCE DURING __skb_unlink()

In the function call, __skb_try_recv_from_queue() (net/core/datagram.c), 
sbk_queue_walk() walks through the queue without checking if the next member in the queue has valid next pointer/address. When a socket buffer has to unlink, __skb_unlink() is called.



Inside __skb_unlink() function, it doesn't verify if skb->next has a valid address. skb->next is assigned and used, without verifying the value inside it. 



What could be probable solution, in this scenario? Should we check if skb->next is not NULL, before calling __skb_unlink()?


--------------------------------------------------------------------------
--------------------------------------------------------------------------


net/core/datagram.c

struct sk_buff *__skb_try_recv_from_queue(struct sock *sk, struct sk_buff_head *queue, unsigned int flags,void (*destructor)(struct sock *sk, struct sk_buff *skb),int *peeked, int *off, int *err, struct sk_buff **last)
{
...
...
...
skb_queue_walk(queue, skb) {
   ...
   ...
   ...
    } else {
==>   __skb_unlink(skb, queue);
       if(destructor){
       ....
...
...
}

Re: Fw: [Bug 202561] BUG: Null pointer dereference in __skb_unlink()

[Cong Wang]

On Tue, Feb 12, 2019 at 6:10 PM Stephen Hemminger
<stephen@networkplumber.org> wrote:
> Original report from sharathkernel@gmail.com:
>
> NULL POINTER DEFERENCE DURING __skb_unlink()
>
> In the function call, __skb_try_recv_from_queue() (net/core/datagram.c),
> sbk_queue_walk() walks through the queue without checking if the next member in the queue has valid next pointer/address. When a socket buffer has to unlink, __skb_unlink() is called.
>
>
>
> Inside __skb_unlink() function, it doesn't verify if skb->next has a valid address. skb->next is assigned and used, without verifying the value inside it.


It should always have a valid ->next pointer as it is in a doubly
linked list, where the last one simply points to the head of the
list. I don't see any problem in the code you quote here.


>
> What could be probable solution, in this scenario? Should we check if skb->next is not NULL, before calling __skb_unlink()?


Do you have a reproducer? Also, your crash report is incomplete,
it doesn't even show a kernel version... Is it 4.20.7? Is it tainted?
Please share the complete dmesg.


Thanks.