I have this emulated device in QEMU, -- and I agree with you that probably no one has been using it for a while IMHO, given the quality of the driver it also make sense to drop the support completely or we at least need to fix some obvious issues here. Best, - Tong
On Mon, Mar 8, 2021 at 1:06 PM Alexander Duyck <alexander.du...@gmail.com> wrote: > > Hi Tong, > > Is this direct-assigned hardware or is QEMU being used to emulate the > hardware here? Admittedly I don't know that much about ATM, so I am > not sure when/if those phys would have gone out of production. However > since the code dates back to 2005 I am guessing it is on the old side. > > Ultimately the decision is up to Chas. However if there has been code > in place for this long that would trigger this kind of null pointer > dereference then it kind of points to the fact that those phys have > probably not been in use since at least back when Linus switched over > to git in 2005. > > Thanks, > > - Alex > > On Mon, Mar 8, 2021 at 9:55 AM Tong Zhang <ztong0...@gmail.com> wrote: > > > > Hi Alex, > > attached is the kernel log for zatm(uPD98402) -- I also have > > idt77252's log -- which is similar to this one -- > > I think it makes sense to drop if no one is actually using it -- > > - Tong > > > > [ 5.740774] BUG: KASAN: null-ptr-deref in uPD98402_start+0x5e/0x219 > > [uPD98402] > > [ 5.741179] Write of size 4 at addr 000000000000002c by task modprobe/96 > > [ 5.741548] > > [ 5.741637] CPU: 0 PID: 96 Comm: modprobe Not tainted 5.12.0-rc2-dirty > > #71 > > [ 5.742017] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), > > BIOS rel-1.13.0-48-gd9c812dda519-prebuilt.qemu.org 04/01/2014 > > [ 5.742635] Call Trace: > > [ 5.742775] dump_stack+0x8a/0xb5 > > [ 5.742966] kasan_report.cold+0x10f/0x111 > > [ 5.743197] ? uPD98402_start+0x5e/0x219 [uPD98402] > > [ 5.743473] uPD98402_start+0x5e/0x219 [uPD98402] > > [ 5.743739] zatm_init_one+0x10b5/0x1311 [zatm] > > [ 5.743998] ? zatm_int.cold+0x30/0x30 [zatm] > > [ 5.744246] ? _raw_write_lock_irqsave+0xd0/0xd0 > > [ 5.744507] ? __mutex_lock_slowpath+0x10/0x10 > > [ 5.744757] ? _raw_spin_unlock_irqrestore+0xd/0x20 > > [ 5.745030] ? zatm_int.cold+0x30/0x30 [zatm] > > [ 5.745278] local_pci_probe+0x6f/0xb0 > > [ 5.745492] pci_device_probe+0x171/0x240 > > [ 5.745718] ? pci_device_remove+0xe0/0xe0 > > [ 5.745949] ? kernfs_create_link+0xb6/0x110 > > [ 5.746190] ? sysfs_do_create_link_sd.isra.0+0x76/0xe0 > > [ 5.746482] really_probe+0x161/0x420 > > [ 5.746691] driver_probe_device+0x6d/0xd0 > > [ 5.746923] device_driver_attach+0x82/0x90 > > [ 5.747158] ? device_driver_attach+0x90/0x90 > > [ 5.747402] __driver_attach+0x60/0x100 > > [ 5.747621] ? device_driver_attach+0x90/0x90 > > [ 5.747864] bus_for_each_dev+0xe1/0x140 > > [ 5.748075] ? subsys_dev_iter_exit+0x10/0x10 > > [ 5.748320] ? klist_node_init+0x61/0x80 > > [ 5.748542] bus_add_driver+0x254/0x2a0 > > [ 5.748760] driver_register+0xd3/0x150 > > [ 5.748977] ? 0xffffffffc0030000 > > [ 5.749163] do_one_initcall+0x84/0x250 > > [ 5.749380] ? trace_event_raw_event_initcall_finish+0x150/0x150 > > [ 5.749714] ? _raw_spin_unlock_irqrestore+0xd/0x20 > > [ 5.749987] ? create_object+0x395/0x510 > > [ 5.750210] ? kasan_unpoison+0x21/0x50 > > [ 5.750427] do_init_module+0xf8/0x350 > > [ 5.750640] load_module+0x40c5/0x4410 > > [ 5.750854] ? module_frob_arch_sections+0x20/0x20 > > [ 5.751123] ? kernel_read_file+0x1cd/0x3e0 > > [ 5.751364] ? __do_sys_finit_module+0x108/0x170 > > [ 5.751628] __do_sys_finit_module+0x108/0x170 > > [ 5.751879] ? __ia32_sys_init_module+0x40/0x40 > > [ 5.752126] ? file_open_root+0x200/0x200 > > [ 5.752353] ? do_sys_open+0x85/0xe0 > > [ 5.752556] ? filp_open+0x50/0x50 > > [ 5.752750] ? fpregs_assert_state_consistent+0x4d/0x60 > > [ 5.753042] ? exit_to_user_mode_prepare+0x2f/0x130 > > [ 5.753316] do_syscall_64+0x33/0x40 > > [ 5.753519] entry_SYSCALL_64_after_hwframe+0x44/0xae > > [ 5.753802] RIP: 0033:0x7ff64032dcf7 > > ff c3 48 c7 c6 01 00 00 00 e9 a1 > > [ 5.755029] RSP: 002b:00007ffd250ea358 EFLAGS: 00000246 ORIG_RAX: > > 0000000000000139 > > [ 5.755449] RAX: ffffffffffffffda RBX: 0000000001093a70 RCX: > > 00007ff64032dcf7 > > [ 5.755847] RDX: 0000000000000000 RSI: 00000000010929e0 RDI: > > 0000000000000003 > > [ 5.756242] RBP: 0000000000000003 R08: 0000000000000000 R09: > > 0000000000000001 > > [ 5.756635] R10: 00007ff640391300 R11: 0000000000000246 R12: > > 00000000010929e0 > > [ 5.757029] R13: 0000000000000000 R14: 0000000001092dd0 R15: > > 0000000000000001 > > > > On Mon, Mar 8, 2021 at 12:47 PM Alexander Duyck > > <alexander.du...@gmail.com> wrote: > > > > > > On Mon, Mar 8, 2021 at 12:39 AM Tong Zhang <ztong0...@gmail.com> wrote: > > > > > > > > there are two drivers(zatm and idt77252) using PRIV() (i.e. > > > > atm->phy_data) > > > > to store private data, but the driver happens to populate wrong > > > > pointers: atm->dev_data. which actually cause null-ptr-dereference in > > > > following PRIV(dev). This patch series attemps to fix those two issues > > > > along with a typo in atm struct. > > > > > > > > Tong Zhang (3): > > > > atm: fix a typo in the struct description > > > > atm: uPD98402: fix incorrect allocation > > > > atm: idt77252: fix null-ptr-dereference > > > > > > > > drivers/atm/idt77105.c | 4 ++-- > > > > drivers/atm/uPD98402.c | 2 +- > > > > include/linux/atmdev.h | 2 +- > > > > 3 files changed, 4 insertions(+), 4 deletions(-) > > > > > > For the 2 phys you actually seen null pointer dereferences or are your > > > changes based on just code review? > > > > > > I ask because it seems like this code has been this way since 2005 and > > > in the case of uPD98402_start the code doesn't seem like it should > > > function the way it was as PRIV is phy_data and there being issues > > > seems pretty obvious since the initialization of things happens > > > immediately after the allocation. > > > > > > I'm just wondering if it might make more sense to drop the code if it > > > hasn't been run in 15+ years rather than updating it?