| Message ID | 20190717085707.9529-1-stefan.bader@canonical.com |
|---|---|
| State | New |
| Headers | show |
| Series | [Xenial,REGN] brcmfmac: add eth_type_trans back for PCIe full dongle | expand |
On 7/17/19 10:57 AM, Stefan Bader wrote: > From: Franky Lin <franky.lin@broadcom.com> > > BugLink: https://bugs.launchpad.net/bugs/1836801 > > A regression was introduced in commit 9c349892ccc9 ("brcmfmac: revise > handling events in receive path") which moves eth_type_trans() call > to brcmf_rx_frame(). Msgbuf layer doesn't use brcmf_rx_frame() but invokes > brcmf_netif_rx() directly. In such case the Ethernet header was not > stripped out resulting in null pointer dereference in the networking > stack. > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000048 > IP: [<ffffffff814c3ce6>] enqueue_to_backlog+0x56/0x260 > PGD 0 > Oops: 0000 [#1] PREEMPT SMP > Modules linked in: fuse ipt_MASQUERADE nf_nat_masquerade_ipv4 > iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 xt_addrtype > [...] > rtsx_pci scsi_mod usbcore usb_common i8042 serio nvme nvme_core > CPU: 7 PID: 1340 Comm: irq/136-brcmf_p Not tainted 4.7.0-rc1-mainline #1 > Hardware name: Dell Inc. XPS 15 9550/0N7TVV, BIOS 01.02.00 04/07/2016 > task: ffff8804a0c5bd00 ti: ffff88049e124000 task.ti: ffff88049e124000 > RIP: 0010:[<ffffffff814c3ce6>] [<ffffffff814c3ce6>] > enqueue_to_backlog+0x56/0x260 > RSP: 0018:ffff88049e127ca0 EFLAGS: 00010046 > RAX: 0000000000000000 RBX: ffff8804bddd7c40 RCX: 000000000000002f > RDX: 0000000000000000 RSI: 0000000000000007 RDI: ffff8804bddd7d4c > RBP: ffff88049e127ce8 R08: 0000000000000000 R09: 0000000000000000 > R10: ffff8804bddd12c0 R11: 000000000000149e R12: 0000000000017c40 > R13: ffff88049e127d08 R14: ffff8804a9bd6d00 R15: ffff8804bddd7d4c > FS: 0000000000000000(0000) GS:ffff8804bddc0000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000048 CR3: 0000000001806000 CR4: 00000000003406e0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Stack: > ffff8804bdddad00 ffff8804ad089e00 0000000000000000 0000000000000282 > 0000000000000000 ffff8804a9bd6d00 ffff8804a1b27e00 ffff8804a9bd6d00 > ffff88002ee88000 ffff88049e127d28 ffffffff814c3f3b ffffffff81311fc3 > Call Trace: > [<ffffffff814c3f3b>] netif_rx_internal+0x4b/0x170 > [<ffffffff81311fc3>] ? swiotlb_tbl_unmap_single+0xf3/0x120 > [<ffffffff814c5467>] netif_rx_ni+0x27/0xc0 > [<ffffffffa08519e9>] brcmf_netif_rx+0x49/0x70 [brcmfmac] > [<ffffffffa08564d4>] brcmf_msgbuf_process_rx+0x2b4/0x570 [brcmfmac] > [<ffffffff81020017>] ? __xen_set_pgd_hyper+0x57/0xd0 > [<ffffffff810d60b0>] ? irq_forced_thread_fn+0x70/0x70 > [<ffffffffa0857381>] brcmf_proto_msgbuf_rx_trigger+0x31/0xe0 [brcmfmac] > [<ffffffffa0861e8f>] brcmf_pcie_isr_thread+0x7f/0x110 [brcmfmac] > [<ffffffff810d60d0>] irq_thread_fn+0x20/0x50 > [<ffffffff810d63ad>] irq_thread+0x12d/0x1c0 > [<ffffffff815d07d5>] ? __schedule+0x2f5/0x7a0 > [<ffffffff810d61d0>] ? wake_threads_waitq+0x30/0x30 > [<ffffffff810d6280>] ? irq_thread_dtor+0xb0/0xb0 > [<ffffffff81098ea8>] kthread+0xd8/0xf0 > [<ffffffff815d4b7f>] ret_from_fork+0x1f/0x40 > [<ffffffff81098dd0>] ? kthread_worker_fn+0x170/0x170 > Code: 1c f5 60 9a 8e 81 9c 58 0f 1f 44 00 00 48 89 45 d0 fa 66 0f 1f > 44 00 00 4c 8d bb 0c 01 00 00 4c 89 ff e8 5e 08 11 00 49 8b 56 20 <48> > 8b 52 48 83 e2 01 74 10 8b 8b 08 01 00 00 8b 15 59 c5 42 00 > RIP [<ffffffff814c3ce6>] enqueue_to_backlog+0x56/0x260 > RSP <ffff88049e127ca0> > CR2: 0000000000000048 > > Fixes: 9c349892ccc9 ("brcmfmac: revise handling events in receive path") > Reported-by: Rafal Milecki <zajec5@gmail.com> > Reported-by: Grey Christoforo <grey@christoforo.net> > Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com> > Reviewed-by: Arend Van Spriel <arend@broadcom.com> > Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com> > Signed-off-by: Franky Lin <franky.lin@broadcom.com> > [arend@broadcom.com: rephrased the commit message] > Signed-off-by: Arend van Spriel <arend@broadcom.com> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> > > (backported from commit 31143e2933d1675c4c1ba6ce125cdd95870edd85) > [smb: file -> drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c > brcmf_msgbuf_process_rx_complete -> brcmf_msgbuf_process_rx] Did the function name really need adjustment? Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > --- > drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c > index 6f7138cea555..f944f356d9c5 100644 > --- a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c > +++ b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c > @@ -1155,6 +1155,8 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf) > brcmu_pkt_buf_free_skb(skb); > return; > } > + > + skb->protocol = eth_type_trans(skb, ifp->ndev); > brcmf_netif_rx(ifp, skb); > } > >
On 17.07.19 11:29, Kleber Souza wrote: > On 7/17/19 10:57 AM, Stefan Bader wrote: >> From: Franky Lin <franky.lin@broadcom.com> >> >> BugLink: https://bugs.launchpad.net/bugs/1836801 >> >> A regression was introduced in commit 9c349892ccc9 ("brcmfmac: revise >> handling events in receive path") which moves eth_type_trans() call >> to brcmf_rx_frame(). Msgbuf layer doesn't use brcmf_rx_frame() but invokes >> brcmf_netif_rx() directly. In such case the Ethernet header was not >> stripped out resulting in null pointer dereference in the networking >> stack. >> >> BUG: unable to handle kernel NULL pointer dereference at 0000000000000048 >> IP: [<ffffffff814c3ce6>] enqueue_to_backlog+0x56/0x260 >> PGD 0 >> Oops: 0000 [#1] PREEMPT SMP >> Modules linked in: fuse ipt_MASQUERADE nf_nat_masquerade_ipv4 >> iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 xt_addrtype >> [...] >> rtsx_pci scsi_mod usbcore usb_common i8042 serio nvme nvme_core >> CPU: 7 PID: 1340 Comm: irq/136-brcmf_p Not tainted 4.7.0-rc1-mainline #1 >> Hardware name: Dell Inc. XPS 15 9550/0N7TVV, BIOS 01.02.00 04/07/2016 >> task: ffff8804a0c5bd00 ti: ffff88049e124000 task.ti: ffff88049e124000 >> RIP: 0010:[<ffffffff814c3ce6>] [<ffffffff814c3ce6>] >> enqueue_to_backlog+0x56/0x260 >> RSP: 0018:ffff88049e127ca0 EFLAGS: 00010046 >> RAX: 0000000000000000 RBX: ffff8804bddd7c40 RCX: 000000000000002f >> RDX: 0000000000000000 RSI: 0000000000000007 RDI: ffff8804bddd7d4c >> RBP: ffff88049e127ce8 R08: 0000000000000000 R09: 0000000000000000 >> R10: ffff8804bddd12c0 R11: 000000000000149e R12: 0000000000017c40 >> R13: ffff88049e127d08 R14: ffff8804a9bd6d00 R15: ffff8804bddd7d4c >> FS: 0000000000000000(0000) GS:ffff8804bddc0000(0000) knlGS:0000000000000000 >> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >> CR2: 0000000000000048 CR3: 0000000001806000 CR4: 00000000003406e0 >> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 >> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 >> Stack: >> ffff8804bdddad00 ffff8804ad089e00 0000000000000000 0000000000000282 >> 0000000000000000 ffff8804a9bd6d00 ffff8804a1b27e00 ffff8804a9bd6d00 >> ffff88002ee88000 ffff88049e127d28 ffffffff814c3f3b ffffffff81311fc3 >> Call Trace: >> [<ffffffff814c3f3b>] netif_rx_internal+0x4b/0x170 >> [<ffffffff81311fc3>] ? swiotlb_tbl_unmap_single+0xf3/0x120 >> [<ffffffff814c5467>] netif_rx_ni+0x27/0xc0 >> [<ffffffffa08519e9>] brcmf_netif_rx+0x49/0x70 [brcmfmac] >> [<ffffffffa08564d4>] brcmf_msgbuf_process_rx+0x2b4/0x570 [brcmfmac] >> [<ffffffff81020017>] ? __xen_set_pgd_hyper+0x57/0xd0 >> [<ffffffff810d60b0>] ? irq_forced_thread_fn+0x70/0x70 >> [<ffffffffa0857381>] brcmf_proto_msgbuf_rx_trigger+0x31/0xe0 [brcmfmac] >> [<ffffffffa0861e8f>] brcmf_pcie_isr_thread+0x7f/0x110 [brcmfmac] >> [<ffffffff810d60d0>] irq_thread_fn+0x20/0x50 >> [<ffffffff810d63ad>] irq_thread+0x12d/0x1c0 >> [<ffffffff815d07d5>] ? __schedule+0x2f5/0x7a0 >> [<ffffffff810d61d0>] ? wake_threads_waitq+0x30/0x30 >> [<ffffffff810d6280>] ? irq_thread_dtor+0xb0/0xb0 >> [<ffffffff81098ea8>] kthread+0xd8/0xf0 >> [<ffffffff815d4b7f>] ret_from_fork+0x1f/0x40 >> [<ffffffff81098dd0>] ? kthread_worker_fn+0x170/0x170 >> Code: 1c f5 60 9a 8e 81 9c 58 0f 1f 44 00 00 48 89 45 d0 fa 66 0f 1f >> 44 00 00 4c 8d bb 0c 01 00 00 4c 89 ff e8 5e 08 11 00 49 8b 56 20 <48> >> 8b 52 48 83 e2 01 74 10 8b 8b 08 01 00 00 8b 15 59 c5 42 00 >> RIP [<ffffffff814c3ce6>] enqueue_to_backlog+0x56/0x260 >> RSP <ffff88049e127ca0> >> CR2: 0000000000000048 >> >> Fixes: 9c349892ccc9 ("brcmfmac: revise handling events in receive path") >> Reported-by: Rafal Milecki <zajec5@gmail.com> >> Reported-by: Grey Christoforo <grey@christoforo.net> >> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com> >> Reviewed-by: Arend Van Spriel <arend@broadcom.com> >> Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com> >> Signed-off-by: Franky Lin <franky.lin@broadcom.com> >> [arend@broadcom.com: rephrased the commit message] >> Signed-off-by: Arend van Spriel <arend@broadcom.com> >> Signed-off-by: Kalle Valo <kvalo@codeaurora.org> >> >> (backported from commit 31143e2933d1675c4c1ba6ce125cdd95870edd85) >> [smb: file -> drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c >> brcmf_msgbuf_process_rx_complete -> brcmf_msgbuf_process_rx] > > Did the function name really need adjustment? I meant to say I applied the code to a different file and a different function than in the original patch. -Stefan > > Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> > > >> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> >> --- >> drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c | 2 ++ >> 1 file changed, 2 insertions(+) >> >> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c >> index 6f7138cea555..f944f356d9c5 100644 >> --- a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c >> +++ b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c >> @@ -1155,6 +1155,8 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf) >> brcmu_pkt_buf_free_skb(skb); >> return; >> } >> + >> + skb->protocol = eth_type_trans(skb, ifp->ndev); >> brcmf_netif_rx(ifp, skb); >> } >> >> >
On Wed, Jul 17, 2019 at 10:57:07AM +0200, Stefan Bader wrote: > From: Franky Lin <franky.lin@broadcom.com> > > BugLink: https://bugs.launchpad.net/bugs/1836801 > > A regression was introduced in commit 9c349892ccc9 ("brcmfmac: revise > handling events in receive path") which moves eth_type_trans() call > to brcmf_rx_frame(). Msgbuf layer doesn't use brcmf_rx_frame() but invokes > brcmf_netif_rx() directly. In such case the Ethernet header was not > stripped out resulting in null pointer dereference in the networking > stack. > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000048 > IP: [<ffffffff814c3ce6>] enqueue_to_backlog+0x56/0x260 > PGD 0 > Oops: 0000 [#1] PREEMPT SMP > Modules linked in: fuse ipt_MASQUERADE nf_nat_masquerade_ipv4 > iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 xt_addrtype > [...] > rtsx_pci scsi_mod usbcore usb_common i8042 serio nvme nvme_core > CPU: 7 PID: 1340 Comm: irq/136-brcmf_p Not tainted 4.7.0-rc1-mainline #1 > Hardware name: Dell Inc. XPS 15 9550/0N7TVV, BIOS 01.02.00 04/07/2016 > task: ffff8804a0c5bd00 ti: ffff88049e124000 task.ti: ffff88049e124000 > RIP: 0010:[<ffffffff814c3ce6>] [<ffffffff814c3ce6>] > enqueue_to_backlog+0x56/0x260 > RSP: 0018:ffff88049e127ca0 EFLAGS: 00010046 > RAX: 0000000000000000 RBX: ffff8804bddd7c40 RCX: 000000000000002f > RDX: 0000000000000000 RSI: 0000000000000007 RDI: ffff8804bddd7d4c > RBP: ffff88049e127ce8 R08: 0000000000000000 R09: 0000000000000000 > R10: ffff8804bddd12c0 R11: 000000000000149e R12: 0000000000017c40 > R13: ffff88049e127d08 R14: ffff8804a9bd6d00 R15: ffff8804bddd7d4c > FS: 0000000000000000(0000) GS:ffff8804bddc0000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000048 CR3: 0000000001806000 CR4: 00000000003406e0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Stack: > ffff8804bdddad00 ffff8804ad089e00 0000000000000000 0000000000000282 > 0000000000000000 ffff8804a9bd6d00 ffff8804a1b27e00 ffff8804a9bd6d00 > ffff88002ee88000 ffff88049e127d28 ffffffff814c3f3b ffffffff81311fc3 > Call Trace: > [<ffffffff814c3f3b>] netif_rx_internal+0x4b/0x170 > [<ffffffff81311fc3>] ? swiotlb_tbl_unmap_single+0xf3/0x120 > [<ffffffff814c5467>] netif_rx_ni+0x27/0xc0 > [<ffffffffa08519e9>] brcmf_netif_rx+0x49/0x70 [brcmfmac] > [<ffffffffa08564d4>] brcmf_msgbuf_process_rx+0x2b4/0x570 [brcmfmac] > [<ffffffff81020017>] ? __xen_set_pgd_hyper+0x57/0xd0 > [<ffffffff810d60b0>] ? irq_forced_thread_fn+0x70/0x70 > [<ffffffffa0857381>] brcmf_proto_msgbuf_rx_trigger+0x31/0xe0 [brcmfmac] > [<ffffffffa0861e8f>] brcmf_pcie_isr_thread+0x7f/0x110 [brcmfmac] > [<ffffffff810d60d0>] irq_thread_fn+0x20/0x50 > [<ffffffff810d63ad>] irq_thread+0x12d/0x1c0 > [<ffffffff815d07d5>] ? __schedule+0x2f5/0x7a0 > [<ffffffff810d61d0>] ? wake_threads_waitq+0x30/0x30 > [<ffffffff810d6280>] ? irq_thread_dtor+0xb0/0xb0 > [<ffffffff81098ea8>] kthread+0xd8/0xf0 > [<ffffffff815d4b7f>] ret_from_fork+0x1f/0x40 > [<ffffffff81098dd0>] ? kthread_worker_fn+0x170/0x170 > Code: 1c f5 60 9a 8e 81 9c 58 0f 1f 44 00 00 48 89 45 d0 fa 66 0f 1f > 44 00 00 4c 8d bb 0c 01 00 00 4c 89 ff e8 5e 08 11 00 49 8b 56 20 <48> > 8b 52 48 83 e2 01 74 10 8b 8b 08 01 00 00 8b 15 59 c5 42 00 > RIP [<ffffffff814c3ce6>] enqueue_to_backlog+0x56/0x260 > RSP <ffff88049e127ca0> > CR2: 0000000000000048 > > Fixes: 9c349892ccc9 ("brcmfmac: revise handling events in receive path") > Reported-by: Rafal Milecki <zajec5@gmail.com> > Reported-by: Grey Christoforo <grey@christoforo.net> > Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com> > Reviewed-by: Arend Van Spriel <arend@broadcom.com> > Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com> > Signed-off-by: Franky Lin <franky.lin@broadcom.com> > [arend@broadcom.com: rephrased the commit message] > Signed-off-by: Arend van Spriel <arend@broadcom.com> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> > > (backported from commit 31143e2933d1675c4c1ba6ce125cdd95870edd85) > [smb: file -> drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c > brcmf_msgbuf_process_rx_complete -> brcmf_msgbuf_process_rx] > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > --- > drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c > index 6f7138cea555..f944f356d9c5 100644 > --- a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c > +++ b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c > @@ -1155,6 +1155,8 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf) > brcmu_pkt_buf_free_skb(skb); > return; > } > + > + skb->protocol = eth_type_trans(skb, ifp->ndev); > brcmf_netif_rx(ifp, skb); > } > This looks to do what is claimed. With the backport data sorted: Acked-by: Andy Whitcroft <apw@canonical.com>
On 17.07.19 11:32, Stefan Bader wrote: > On 17.07.19 11:29, Kleber Souza wrote: >> On 7/17/19 10:57 AM, Stefan Bader wrote: >>> From: Franky Lin <franky.lin@broadcom.com> >>> >>> BugLink: https://bugs.launchpad.net/bugs/1836801 >>> >>> A regression was introduced in commit 9c349892ccc9 ("brcmfmac: revise >>> handling events in receive path") which moves eth_type_trans() call >>> to brcmf_rx_frame(). Msgbuf layer doesn't use brcmf_rx_frame() but invokes >>> brcmf_netif_rx() directly. In such case the Ethernet header was not >>> stripped out resulting in null pointer dereference in the networking >>> stack. >>> >>> BUG: unable to handle kernel NULL pointer dereference at 0000000000000048 >>> IP: [<ffffffff814c3ce6>] enqueue_to_backlog+0x56/0x260 >>> PGD 0 >>> Oops: 0000 [#1] PREEMPT SMP >>> Modules linked in: fuse ipt_MASQUERADE nf_nat_masquerade_ipv4 >>> iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 xt_addrtype >>> [...] >>> rtsx_pci scsi_mod usbcore usb_common i8042 serio nvme nvme_core >>> CPU: 7 PID: 1340 Comm: irq/136-brcmf_p Not tainted 4.7.0-rc1-mainline #1 >>> Hardware name: Dell Inc. XPS 15 9550/0N7TVV, BIOS 01.02.00 04/07/2016 >>> task: ffff8804a0c5bd00 ti: ffff88049e124000 task.ti: ffff88049e124000 >>> RIP: 0010:[<ffffffff814c3ce6>] [<ffffffff814c3ce6>] >>> enqueue_to_backlog+0x56/0x260 >>> RSP: 0018:ffff88049e127ca0 EFLAGS: 00010046 >>> RAX: 0000000000000000 RBX: ffff8804bddd7c40 RCX: 000000000000002f >>> RDX: 0000000000000000 RSI: 0000000000000007 RDI: ffff8804bddd7d4c >>> RBP: ffff88049e127ce8 R08: 0000000000000000 R09: 0000000000000000 >>> R10: ffff8804bddd12c0 R11: 000000000000149e R12: 0000000000017c40 >>> R13: ffff88049e127d08 R14: ffff8804a9bd6d00 R15: ffff8804bddd7d4c >>> FS: 0000000000000000(0000) GS:ffff8804bddc0000(0000) knlGS:0000000000000000 >>> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 >>> CR2: 0000000000000048 CR3: 0000000001806000 CR4: 00000000003406e0 >>> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 >>> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 >>> Stack: >>> ffff8804bdddad00 ffff8804ad089e00 0000000000000000 0000000000000282 >>> 0000000000000000 ffff8804a9bd6d00 ffff8804a1b27e00 ffff8804a9bd6d00 >>> ffff88002ee88000 ffff88049e127d28 ffffffff814c3f3b ffffffff81311fc3 >>> Call Trace: >>> [<ffffffff814c3f3b>] netif_rx_internal+0x4b/0x170 >>> [<ffffffff81311fc3>] ? swiotlb_tbl_unmap_single+0xf3/0x120 >>> [<ffffffff814c5467>] netif_rx_ni+0x27/0xc0 >>> [<ffffffffa08519e9>] brcmf_netif_rx+0x49/0x70 [brcmfmac] >>> [<ffffffffa08564d4>] brcmf_msgbuf_process_rx+0x2b4/0x570 [brcmfmac] >>> [<ffffffff81020017>] ? __xen_set_pgd_hyper+0x57/0xd0 >>> [<ffffffff810d60b0>] ? irq_forced_thread_fn+0x70/0x70 >>> [<ffffffffa0857381>] brcmf_proto_msgbuf_rx_trigger+0x31/0xe0 [brcmfmac] >>> [<ffffffffa0861e8f>] brcmf_pcie_isr_thread+0x7f/0x110 [brcmfmac] >>> [<ffffffff810d60d0>] irq_thread_fn+0x20/0x50 >>> [<ffffffff810d63ad>] irq_thread+0x12d/0x1c0 >>> [<ffffffff815d07d5>] ? __schedule+0x2f5/0x7a0 >>> [<ffffffff810d61d0>] ? wake_threads_waitq+0x30/0x30 >>> [<ffffffff810d6280>] ? irq_thread_dtor+0xb0/0xb0 >>> [<ffffffff81098ea8>] kthread+0xd8/0xf0 >>> [<ffffffff815d4b7f>] ret_from_fork+0x1f/0x40 >>> [<ffffffff81098dd0>] ? kthread_worker_fn+0x170/0x170 >>> Code: 1c f5 60 9a 8e 81 9c 58 0f 1f 44 00 00 48 89 45 d0 fa 66 0f 1f >>> 44 00 00 4c 8d bb 0c 01 00 00 4c 89 ff e8 5e 08 11 00 49 8b 56 20 <48> >>> 8b 52 48 83 e2 01 74 10 8b 8b 08 01 00 00 8b 15 59 c5 42 00 >>> RIP [<ffffffff814c3ce6>] enqueue_to_backlog+0x56/0x260 >>> RSP <ffff88049e127ca0> >>> CR2: 0000000000000048 >>> >>> Fixes: 9c349892ccc9 ("brcmfmac: revise handling events in receive path") >>> Reported-by: Rafal Milecki <zajec5@gmail.com> >>> Reported-by: Grey Christoforo <grey@christoforo.net> >>> Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com> >>> Reviewed-by: Arend Van Spriel <arend@broadcom.com> >>> Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com> >>> Signed-off-by: Franky Lin <franky.lin@broadcom.com> >>> [arend@broadcom.com: rephrased the commit message] >>> Signed-off-by: Arend van Spriel <arend@broadcom.com> >>> Signed-off-by: Kalle Valo <kvalo@codeaurora.org> >>> >>> (backported from commit 31143e2933d1675c4c1ba6ce125cdd95870edd85) >>> [smb: file -> drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c >>> brcmf_msgbuf_process_rx_complete -> brcmf_msgbuf_process_rx] Oh ok, I did get confused by the stack trace mentioning the other function but applied the hunk to the correct function by luck. So above should be changed into [smb: file -> drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c] Thanks for catching this. Stefan >> >> Did the function name really need adjustment? > > I meant to say I applied the code to a different file and a different function > than in the original patch. > > -Stefan > >> >> Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com> >> >> >>> Signed-off-by: Stefan Bader <stefan.bader@canonical.com> >>> --- >>> drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c | 2 ++ >>> 1 file changed, 2 insertions(+) >>> >>> diff --git a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c >>> index 6f7138cea555..f944f356d9c5 100644 >>> --- a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c >>> +++ b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c >>> @@ -1155,6 +1155,8 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf) >>> brcmu_pkt_buf_free_skb(skb); >>> return; >>> } >>> + >>> + skb->protocol = eth_type_trans(skb, ifp->ndev); >>> brcmf_netif_rx(ifp, skb); >>> } >>> >>> >> > > >
On 17.07.19 10:57, Stefan Bader wrote: > From: Franky Lin <franky.lin@broadcom.com> > > BugLink: https://bugs.launchpad.net/bugs/1836801 > > A regression was introduced in commit 9c349892ccc9 ("brcmfmac: revise > handling events in receive path") which moves eth_type_trans() call > to brcmf_rx_frame(). Msgbuf layer doesn't use brcmf_rx_frame() but invokes > brcmf_netif_rx() directly. In such case the Ethernet header was not > stripped out resulting in null pointer dereference in the networking > stack. > > BUG: unable to handle kernel NULL pointer dereference at 0000000000000048 > IP: [<ffffffff814c3ce6>] enqueue_to_backlog+0x56/0x260 > PGD 0 > Oops: 0000 [#1] PREEMPT SMP > Modules linked in: fuse ipt_MASQUERADE nf_nat_masquerade_ipv4 > iptable_nat nf_conntrack_ipv4 nf_defrag_ipv4 nf_nat_ipv4 xt_addrtype > [...] > rtsx_pci scsi_mod usbcore usb_common i8042 serio nvme nvme_core > CPU: 7 PID: 1340 Comm: irq/136-brcmf_p Not tainted 4.7.0-rc1-mainline #1 > Hardware name: Dell Inc. XPS 15 9550/0N7TVV, BIOS 01.02.00 04/07/2016 > task: ffff8804a0c5bd00 ti: ffff88049e124000 task.ti: ffff88049e124000 > RIP: 0010:[<ffffffff814c3ce6>] [<ffffffff814c3ce6>] > enqueue_to_backlog+0x56/0x260 > RSP: 0018:ffff88049e127ca0 EFLAGS: 00010046 > RAX: 0000000000000000 RBX: ffff8804bddd7c40 RCX: 000000000000002f > RDX: 0000000000000000 RSI: 0000000000000007 RDI: ffff8804bddd7d4c > RBP: ffff88049e127ce8 R08: 0000000000000000 R09: 0000000000000000 > R10: ffff8804bddd12c0 R11: 000000000000149e R12: 0000000000017c40 > R13: ffff88049e127d08 R14: ffff8804a9bd6d00 R15: ffff8804bddd7d4c > FS: 0000000000000000(0000) GS:ffff8804bddc0000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000048 CR3: 0000000001806000 CR4: 00000000003406e0 > DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000 > DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400 > Stack: > ffff8804bdddad00 ffff8804ad089e00 0000000000000000 0000000000000282 > 0000000000000000 ffff8804a9bd6d00 ffff8804a1b27e00 ffff8804a9bd6d00 > ffff88002ee88000 ffff88049e127d28 ffffffff814c3f3b ffffffff81311fc3 > Call Trace: > [<ffffffff814c3f3b>] netif_rx_internal+0x4b/0x170 > [<ffffffff81311fc3>] ? swiotlb_tbl_unmap_single+0xf3/0x120 > [<ffffffff814c5467>] netif_rx_ni+0x27/0xc0 > [<ffffffffa08519e9>] brcmf_netif_rx+0x49/0x70 [brcmfmac] > [<ffffffffa08564d4>] brcmf_msgbuf_process_rx+0x2b4/0x570 [brcmfmac] > [<ffffffff81020017>] ? __xen_set_pgd_hyper+0x57/0xd0 > [<ffffffff810d60b0>] ? irq_forced_thread_fn+0x70/0x70 > [<ffffffffa0857381>] brcmf_proto_msgbuf_rx_trigger+0x31/0xe0 [brcmfmac] > [<ffffffffa0861e8f>] brcmf_pcie_isr_thread+0x7f/0x110 [brcmfmac] > [<ffffffff810d60d0>] irq_thread_fn+0x20/0x50 > [<ffffffff810d63ad>] irq_thread+0x12d/0x1c0 > [<ffffffff815d07d5>] ? __schedule+0x2f5/0x7a0 > [<ffffffff810d61d0>] ? wake_threads_waitq+0x30/0x30 > [<ffffffff810d6280>] ? irq_thread_dtor+0xb0/0xb0 > [<ffffffff81098ea8>] kthread+0xd8/0xf0 > [<ffffffff815d4b7f>] ret_from_fork+0x1f/0x40 > [<ffffffff81098dd0>] ? kthread_worker_fn+0x170/0x170 > Code: 1c f5 60 9a 8e 81 9c 58 0f 1f 44 00 00 48 89 45 d0 fa 66 0f 1f > 44 00 00 4c 8d bb 0c 01 00 00 4c 89 ff e8 5e 08 11 00 49 8b 56 20 <48> > 8b 52 48 83 e2 01 74 10 8b 8b 08 01 00 00 8b 15 59 c5 42 00 > RIP [<ffffffff814c3ce6>] enqueue_to_backlog+0x56/0x260 > RSP <ffff88049e127ca0> > CR2: 0000000000000048 > > Fixes: 9c349892ccc9 ("brcmfmac: revise handling events in receive path") > Reported-by: Rafal Milecki <zajec5@gmail.com> > Reported-by: Grey Christoforo <grey@christoforo.net> > Reviewed-by: Pieter-Paul Giesberts <pieter-paul.giesberts@broadcom.com> > Reviewed-by: Arend Van Spriel <arend@broadcom.com> > Reviewed-by: Hante Meuleman <hante.meuleman@broadcom.com> > Signed-off-by: Franky Lin <franky.lin@broadcom.com> > [arend@broadcom.com: rephrased the commit message] > Signed-off-by: Arend van Spriel <arend@broadcom.com> > Signed-off-by: Kalle Valo <kvalo@codeaurora.org> > > (backported from commit 31143e2933d1675c4c1ba6ce125cdd95870edd85) > [smb: file -> drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c > brcmf_msgbuf_process_rx_complete -> brcmf_msgbuf_process_rx] > Signed-off-by: Stefan Bader <stefan.bader@canonical.com> > --- Applied to xenial/master-next and re-spun. Thanks. -Stefan > drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c > index 6f7138cea555..f944f356d9c5 100644 > --- a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c > +++ b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c > @@ -1155,6 +1155,8 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf) > brcmu_pkt_buf_free_skb(skb); > return; > } > + > + skb->protocol = eth_type_trans(skb, ifp->ndev); > brcmf_netif_rx(ifp, skb); > } > >
diff --git a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c index 6f7138cea555..f944f356d9c5 100644 --- a/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c +++ b/drivers/net/wireless/brcm80211/brcmfmac/msgbuf.c @@ -1155,6 +1155,8 @@ brcmf_msgbuf_process_rx_complete(struct brcmf_msgbuf *msgbuf, void *buf) brcmu_pkt_buf_free_skb(skb); return; } + + skb->protocol = eth_type_trans(skb, ifp->ndev); brcmf_netif_rx(ifp, skb); }