linux-kernel - Re: [PATCH] net: check for dev pointer being NULL in dev_hard

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 1 Dec 2022 15:10:11 -0500
From:   George Kennedy <george.kennedy@...cle.com>
To:     Eric Dumazet <edumazet@...gle.com>,
        Pavan Chebbi <pavan.chebbi@...adcom.com>
Cc:     davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org,
        bpf@...r.kernel.org, harshit.m.mogalapalli@...cle.com
Subject: Re: [PATCH] net: check for dev pointer being NULL in
 dev_hard_header() to avoid GPF



On 12/1/2022 2:25 PM, Eric Dumazet wrote:
> On Thu, Dec 1, 2022 at 2:16 PM Pavan Chebbi <pavan.chebbi@...adcom.com> wrote:
>> On Wed, Nov 30, 2022 at 7:43 PM George Kennedy
>> <george.kennedy@...cle.com> wrote:
>>> The dev pointer can be NULL in dev_hard_header(). Add check for dev being
>>> NULL in dev_hard_header() to avoid GPF.
>>>
>>> general protection fault, probably for non-canonical address
>>>      0xdffffc0000000046: 0000 [#1] PREEMPT SMP KASAN NOPTI
>>> KASAN: null-ptr-deref in range [0x0000000000000230-0x0000000000000237]
>>> CPU: 1 PID: 45 Comm: kworker/1:1 Not tainted 6.1.0-rc7+ #2
>>> Hardware name: Red Hat KVM, BIOS 1.15.0-2.module+el8.6.0+20659+3dcf7c70
>>> Workqueue: mld mld_ifc_work
>>> RIP: 0010:macvlan_hard_header (./include/linux/netdevice.h:3057
>>>      (discriminator 4) drivers/net/macvlan.c:594 (discriminator 4))
>>> RSP: 0018:ffff888103d377d0 EFLAGS: 00010212
>>> RAX: dffffc0000000000 RBX: ffff88801cf1a000 RCX: 0000000000000000
>>> RDX: 0000000000000046 RSI: 0000000000000000 RDI: 0000000000000230
>>> RBP: ffff88801e8ef328 R08: 0000000000000000 R09: 0000000000000060
>>> R10: 0000000000000000 R11: 0000000000000000 R12: ffff88801f0497c0
>>> R13: 0000000000000000 R14: ffff888045187c98 R15: 0000000000000060
>>> FS:  0000000000000000(0000) GS:ffff888106c80000(0000)
>>>      knlGS:0000000000000000
>>> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> CR2: 00007fbf3f1c1840 CR3: 0000000014e36000 CR4: 00000000000006e0
>>> Call Trace:
>>>   <TASK>
>>> neigh_connected_output (./include/linux/netdevice.h:3060
>>>      net/core/neighbour.c:1595)
>>> ip6_finish_output2 (./include/net/neighbour.h:546
>>>      net/ipv6/ip6_output.c:134)
>>> ip6_finish_output (net/ipv6/ip6_output.c:195 net/ipv6/ip6_output.c:206)
>>> ip6_output (./include/linux/netfilter.h:291 net/ipv6/ip6_output.c:227)
>>> NF_HOOK.constprop.0 (./include/net/dst.h:445
>>>      ./include/linux/netfilter.h:302)
>>> mld_sendpack (net/ipv6/mcast.c:1824)
>>> mld_send_cr (net/ipv6/mcast.c:2122)
>>> mld_ifc_work (net/ipv6/mcast.c:2655)
>>> process_one_work (kernel/workqueue.c:2294)
>>> worker_thread (./include/linux/list.h:292 kernel/workqueue.c:2437)
>>> kthread (kernel/kthread.c:376)
>>> ret_from_fork (arch/x86/entry/entry_64.S:312)
>>>   </TASK>
>>> Modules linked in:
>>> Dumping ftrace buffer:
>>>     (ftrace buffer empty)
>>> ---[ end trace 0000000000000000 ]---
>>>
>>> Fixes: 0c4e85813d0a ("[NET]: Wrap netdevice hardware header creation.")
>>> Reported-by: syzkaller <syzkaller@...glegroups.com>
>>> Signed-off-by: George Kennedy <george.kennedy@...cle.com>
>>> ---
>>>   include/linux/netdevice.h | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>>> index eddf8ee270e7..9b25a6301fa5 100644
>>> --- a/include/linux/netdevice.h
>>> +++ b/include/linux/netdevice.h
>>> @@ -3054,7 +3054,7 @@ static inline int dev_hard_header(struct sk_buff *skb, struct net_device *dev,
>>>                                    const void *daddr, const void *saddr,
>>>                                    unsigned int len)
>>>   {
>>> -       if (!dev->header_ops || !dev->header_ops->create)
>>> +       if (!dev || !dev->header_ops || !dev->header_ops->create)
> Do  you have a repro ?
See syzkaller repros attached.

> This patch will not prevent a crash later I think.

The repro ran overnight without failure with the patch applied.

>
> Please fix the root cause, thanks !

Will try.

Thanks,
George
>
>>>                  return 0;
>> net_device being NULL during eth header construction? seems like a
>> more serious issue?
>> If it indeed is a genuine scenario I think a better description is needed...
>>
>>>          return dev->header_ops->create(skb, dev, type, daddr, saddr, len);
>>> --
>>> 2.31.1
>>>

View attachment "repro_macvlan1.c" of type "text/plain" (44989 bytes)

View attachment "repro_macvlan2.c" of type "text/plain" (44416 bytes)