On Wed, Aug 21, 2024 at 6:07 PM Dawid Osuchowski <[email protected]> wrote: > > Ethtool callbacks can be executed while reset is in progress and try to > access deleted resources, e.g. getting coalesce settings can result in a > NULL pointer dereference seen below. > > Reproduction steps: > Once the driver is fully initialized, trigger reset: > # echo 1 > /sys/class/net/<interface>/device/reset > when reset is in progress try to get coalesce settings using ethtool: > # ethtool -c <interface> > > BUG: kernel NULL pointer dereference, address: 0000000000000020 > PGD 0 P4D 0 > Oops: Oops: 0000 [#1] PREEMPT SMP PTI > CPU: 11 PID: 19713 Comm: ethtool Tainted: G S 6.10.0-rc7+ #7 > RIP: 0010:ice_get_q_coalesce+0x2e/0xa0 [ice] > RSP: 0018:ffffbab1e9bcf6a8 EFLAGS: 00010206 > RAX: 000000000000000c RBX: ffff94512305b028 RCX: 0000000000000000 > RDX: 0000000000000000 RSI: ffff9451c3f2e588 RDI: ffff9451c3f2e588 > RBP: 0000000000000000 R08: 0000000000000000 R09: 0000000000000000 > R10: ffff9451c3f2e580 R11: 000000000000001f R12: ffff945121fa9000 > R13: ffffbab1e9bcf760 R14: 0000000000000013 R15: ffffffff9e65dd40 > FS: 00007faee5fbe740(0000) GS:ffff94546fd80000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 0000000000000020 CR3: 0000000106c2e005 CR4: 00000000001706f0 > Call Trace: > <TASK> > ice_get_coalesce+0x17/0x30 [ice] > coalesce_prepare_data+0x61/0x80 > ethnl_default_doit+0xde/0x340 > genl_family_rcv_msg_doit+0xf2/0x150 > genl_rcv_msg+0x1b3/0x2c0 > netlink_rcv_skb+0x5b/0x110 > genl_rcv+0x28/0x40 > netlink_unicast+0x19c/0x290 > netlink_sendmsg+0x222/0x490 > __sys_sendto+0x1df/0x1f0 > __x64_sys_sendto+0x24/0x30 > do_syscall_64+0x82/0x160 > entry_SYSCALL_64_after_hwframe+0x76/0x7e > RIP: 0033:0x7faee60d8e27 > > Calling netif_device_detach() before reset makes the net core not call > the driver when ethtool command is issued, the attempt to execute an > ethtool command during reset will result in the following message: > > netlink error: No such device > > instead of NULL pointer dereference. Once reset is done and > ice_rebuild() is executing, the netif_device_attach() is called to allow > for ethtool operations to occur again in a safe manner. > > Fixes: fcea6f3da546 ("ice: Add stats and ethtool support") > Suggested-by: Jakub Kicinski <[email protected]> > Reviewed-by: Igor Bagnucki <[email protected]> > Signed-off-by: Dawid Osuchowski <[email protected]> > --- > Changes since v1: > * Changed Fixes tag to point to another commit > * Minified the stacktrace > > Changes since v2: > * Moved netif_device_attach() directly into ice_rebuild() and perform it > only on main vsi > > Changes since v3: > * Style changes requested by Przemek Kitszel > > Changes since v4: > * Applied reverse xmas tree rule to declaration of ice_vsi *vsi variable > > Suggestion from Kuba: > https://lore.kernel.org/netdev/[email protected]/ > Previous attempt (dropped because it introduced regression with link up): > https://lore.kernel.org/netdev/[email protected]/
This v5 passes the tests that the previous attempt referenced above failed. The patch looks sane. Reviewed-by: Michal Schmidt <[email protected]>
