lkml.org 
[lkml]   [2021]   [Jun]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] net: 3com: 3c59x: add a check against null pointer dereference
From
Date
On 6/12/21 4:56 AM, Zheyu Ma wrote:
> When the driver is processing the interrupt, it will read the value of
> the register to determine the status of the device. If the device is in
> an incorrect state, the driver may mistakenly enter this branch. At this
> time, the dma buffer has not been allocated, which will result in a null
> pointer dereference.
>
> Fix this by checking whether the buffer is allocated.
>
> This log reveals it:
>
> BUG: kernel NULL pointer dereference, address: 0000000000000070
> PGD 0 P4D 0
> Oops: 0000 [#1] PREEMPT SMP PTI
> CPU: 5 PID: 0 Comm: swapper/5 Not tainted 5.12.4-g70e7f0549188-dirty #88
> Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS rel-1.12.0-59-gc9ba5276e321-prebuilt.qemu.org 04/01/2014
> RIP: 0010:_vortex_interrupt+0x323/0x670
> Code: 84 d4 00 00 00 e8 bd e9 60 fe 48 8b 45 d8 48 83 c0 0c 48 89 c6 bf 00 10 00 00 e8 98 d0 f0 fe 48 8b 45 d0 48 8b 80 d8 01 00 00 <8b> 40 70 83 c0 03 89 c0 83 e0 fc 48 89 c2 48 8b 45 d0 48 8b b0 e0
> RSP: 0018:ffffc900001a4dd0 EFLAGS: 00010046
> RAX: 0000000000000000 RBX: ffff888115da0580 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: ffffffff81bf710e RDI: 0000000000001000
> RBP: ffffc900001a4e30 R08: ffff8881008edbc0 R09: 00000000fffffffe
> R10: 0000000000000001 R11: 00000000a5c81234 R12: ffff8881049530a8
> R13: 0000000000000000 R14: ffffffff87313288 R15: ffff888108c92000
> FS: 0000000000000000(0000) GS:ffff88817b200000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000070 CR3: 00000001198c2000 CR4: 00000000000006e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Call Trace:
> <IRQ>
> ? _raw_spin_lock_irqsave+0x81/0xa0
> vortex_boomerang_interrupt+0x56/0xc10
> ? __this_cpu_preempt_check+0x1c/0x20
> __handle_irq_event_percpu+0x58/0x3e0
> handle_irq_event_percpu+0x3a/0x90
> handle_irq_event+0x3e/0x60
> handle_fasteoi_irq+0xc7/0x1d0
> __common_interrupt+0x84/0x150
> common_interrupt+0xb4/0xd0
> </IRQ>
> asm_common_interrupt+0x1e/0x40
> RIP: 0010:native_safe_halt+0x17/0x20
> Code: 07 0f 00 2d 3b 3e 4b 00 f4 5d c3 0f 1f 84 00 00 00 00 00 8b 05 42 a9 72 02 55 48 89 e5 85 c0 7e 07 0f 00 2d 1b 3e 4b 00 fb f4 <5d> c3 cc cc cc cc cc cc cc 0f 1f 44 00 00 55 48 89 e5 e8 92 4a ff
> RSP: 0018:ffffc900000afe90 EFLAGS: 00000246
> RAX: 0000000000000000 RBX: 0000000000000005 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: ffffffff8666cafb RDI: ffffffff865058de
> RBP: ffffc900000afe90 R08: 0000000000000001 R09: 0000000000000001
> R10: 0000000000000001 R11: 0000000000000000 R12: ffffffff87313288
> R13: 0000000000000000 R14: 0000000000000000 R15: ffff8881008ed1c0
> default_idle+0xe/0x20
> arch_cpu_idle+0xf/0x20
> default_idle_call+0x73/0x250
> do_idle+0x1f5/0x2d0
> cpu_startup_entry+0x1d/0x20
> start_secondary+0x11f/0x160
> secondary_startup_64_no_verify+0xb0/0xbb
> Modules linked in:
> Dumping ftrace buffer:
> (ftrace buffer empty)
> CR2: 0000000000000070
> ---[ end trace 0735407a540147e1 ]---
> RIP: 0010:_vortex_interrupt+0x323/0x670
> Code: 84 d4 00 00 00 e8 bd e9 60 fe 48 8b 45 d8 48 83 c0 0c 48 89 c6 bf 00 10 00 00 e8 98 d0 f0 fe 48 8b 45 d0 48 8b 80 d8 01 00 00 <8b> 40 70 83 c0 03 89 c0 83 e0 fc 48 89 c2 48 8b 45 d0 48 8b b0 e0
> RSP: 0018:ffffc900001a4dd0 EFLAGS: 00010046
> RAX: 0000000000000000 RBX: ffff888115da0580 RCX: 0000000000000000
> RDX: 0000000000000000 RSI: ffffffff81bf710e RDI: 0000000000001000
> RBP: ffffc900001a4e30 R08: ffff8881008edbc0 R09: 00000000fffffffe
> R10: 0000000000000001 R11: 00000000a5c81234 R12: ffff8881049530a8
> R13: 0000000000000000 R14: ffffffff87313288 R15: ffff888108c92000
> FS: 0000000000000000(0000) GS:ffff88817b200000(0000) knlGS:0000000000000000
> CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 0000000000000070 CR3: 00000001198c2000 CR4: 00000000000006e0
> DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
> DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
> Kernel panic - not syncing: Fatal exception in interrupt
> Dumping ftrace buffer:
> (ftrace buffer empty)
> Kernel Offset: disabled
> Rebooting in 1 seconds..
>
> Signed-off-by: Zheyu Ma <zheyuma97@gmail.com>
> ---
> drivers/net/ethernet/3com/3c59x.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/3com/3c59x.c b/drivers/net/ethernet/3com/3c59x.c
> index 741c67e546d4..e27901ded7a0 100644
> --- a/drivers/net/ethernet/3com/3c59x.c
> +++ b/drivers/net/ethernet/3com/3c59x.c
> @@ -2300,7 +2300,7 @@ _vortex_interrupt(int irq, struct net_device *dev)
> }
>
> if (status & DMADone) {
> - if (ioread16(ioaddr + Wn7_MasterStatus) & 0x1000) {
> + if ((ioread16(ioaddr + Wn7_MasterStatus) & 0x1000) && vp->tx_skb_dma) {
> iowrite16(0x1000, ioaddr + Wn7_MasterStatus); /* Ack the event. */
> dma_unmap_single(vp->gendev, vp->tx_skb_dma, (vp->tx_skb->len + 3) & ~3, DMA_TO_DEVICE);
> pkts_compl++;

This means you won't be ack'ing the event - is this unacknowledged event
going to cause an issue later?

If the error is because the buffer doesn't exist, then can you simply
put the buffer check on the dma_unmap_single() and allow the rest of the
handling to happen?

sln

\
 
 \ /
  Last update: 2021-06-14 18:50    [W:0.043 / U:0.216 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site