On Tue, Mar 11, 2025 at 03:17:40AM -0700, Shradha Gupta wrote:
> When on a MANA VM hibernation is triggered, as part of hibernate_snapshot(),
> mana_gd_suspend() and mana_gd_resume() are called. If during this
> mana_gd_resume(), a failure occurs with HWC creation, mana_port_debugfs
> pointer does not get reinitialized and ends up pointing to older,
> cleaned-up dentry.
> Further in the hibernation path, as part of power_down(), mana_gd_shutdown()
> is triggered. This call, unaware of the failures in resume, tries to cleanup
> the already cleaned up mana_port_debugfs value and hits the following bug:
>
> [ 191.359296] mana 7870:00:00.0: Shutdown was called
> [ 191.359918] BUG: kernel NULL pointer dereference, address: 0000000000000098
> [ 191.360584] #PF: supervisor write access in kernel mode
> [ 191.361125] #PF: error_code(0x0002) - not-present page
> [ 191.361727] PGD 1080ea067 P4D 0
> [ 191.362172] Oops: Oops: 0002 [#1] SMP NOPTI
> [ 191.362606] CPU: 11 UID: 0 PID: 1674 Comm: bash Not tainted 6.14.0-rc5+ #2
> [ 191.363292] Hardware name: Microsoft Corporation Virtual Machine/Virtual
> Machine, BIOS Hyper-V UEFI Release v4.1 11/21/2024
> [ 191.364124] RIP: 0010:down_write+0x19/0x50
> [ 191.364537] Code: 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f
> 44 00 00 55 48 89 e5 53 48 89 fb e8 de cd ff ff 31 c0 ba 01 00 00 00 <f0> 48
> 0f b1 13 75 16 65 48 8b 05 88 24 4c 6a 48 89 43 08 48 8b 5d
> [ 191.365867] RSP: 0000:ff45fbe0c1c037b8 EFLAGS: 00010246
> [ 191.366350] RAX: 0000000000000000 RBX: 0000000000000098 RCX:
> ffffff8100000000
> [ 191.366951] RDX: 0000000000000001 RSI: 0000000000000064 RDI:
> 0000000000000098
> [ 191.367600] RBP: ff45fbe0c1c037c0 R08: 0000000000000000 R09:
> 0000000000000001
> [ 191.368225] R10: ff45fbe0d2b01000 R11: 0000000000000008 R12:
> 0000000000000000
> [ 191.368874] R13: 000000000000000b R14: ff43dc27509d67c0 R15:
> 0000000000000020
> [ 191.369549] FS: 00007dbc5001e740(0000) GS:ff43dc663f380000(0000)
> knlGS:0000000000000000
> [ 191.370213] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [ 191.370830] CR2: 0000000000000098 CR3: 0000000168e8e002 CR4:
> 0000000000b73ef0
> [ 191.371557] DR0: 0000000000000000 DR1: 0000000000000000 DR2:
> 0000000000000000
> [ 191.372192] DR3: 0000000000000000 DR6: 00000000fffe07f0 DR7:
> 0000000000000400
> [ 191.372906] Call Trace:
> [ 191.373262] <TASK>
> [ 191.373621] ? show_regs+0x64/0x70
> [ 191.374040] ? __die+0x24/0x70
> [ 191.374468] ? page_fault_oops+0x290/0x5b0
> [ 191.374875] ? do_user_addr_fault+0x448/0x800
> [ 191.375357] ? exc_page_fault+0x7a/0x160
> [ 191.375971] ? asm_exc_page_fault+0x27/0x30
> [ 191.376416] ? down_write+0x19/0x50
> [ 191.376832] ? down_write+0x12/0x50
> [ 191.377232] simple_recursive_removal+0x4a/0x2a0
> [ 191.377679] ? __pfx_remove_one+0x10/0x10
> [ 191.378088] debugfs_remove+0x44/0x70
> [ 191.378530] mana_detach+0x17c/0x4f0
> [ 191.378950] ? __flush_work+0x1e2/0x3b0
> [ 191.379362] ? __cond_resched+0x1a/0x50
> [ 191.379787] mana_remove+0xf2/0x1a0
> [ 191.380193] mana_gd_shutdown+0x3b/0x70
> [ 191.380642] pci_device_shutdown+0x3a/0x80
> [ 191.381063] device_shutdown+0x13e/0x230
> [ 191.381480] kernel_power_off+0x35/0x80
> [ 191.381890] hibernate+0x3c6/0x470
> [ 191.382312] state_store+0xcb/0xd0
> [ 191.382734] kobj_attr_store+0x12/0x30
> [ 191.383211] sysfs_kf_write+0x3e/0x50
> [ 191.383640] kernfs_fop_write_iter+0x140/0x1d0
> [ 191.384106] vfs_write+0x271/0x440
> [ 191.384521] ksys_write+0x72/0xf0
> [ 191.384924] __x64_sys_write+0x19/0x20
> [ 191.385313] x64_sys_call+0x2b0/0x20b0
> [ 191.385736] do_syscall_64+0x79/0x150
> [ 191.386146] ? __mod_memcg_lruvec_state+0xe7/0x240
> [ 191.386676] ? __lruvec_stat_mod_folio+0x79/0xb0
> [ 191.387124] ? __pfx_lru_add+0x10/0x10
> [ 191.387515] ? queued_spin_unlock+0x9/0x10
> [ 191.387937] ? do_anonymous_page+0x33c/0xa00
> [ 191.388374] ? __handle_mm_fault+0xcf3/0x1210
> [ 191.388805] ? __count_memcg_events+0xbe/0x180
> [ 191.389235] ? handle_mm_fault+0xae/0x300
> [ 191.389588] ? do_user_addr_fault+0x559/0x800
> [ 191.390027] ? irqentry_exit_to_user_mode+0x43/0x230
> [ 191.390525] ? irqentry_exit+0x1d/0x30
> [ 191.390879] ? exc_page_fault+0x86/0x160
> [ 191.391235] entry_SYSCALL_64_after_hwframe+0x76/0x7e
> [ 191.391745] RIP: 0033:0x7dbc4ff1c574
> [ 191.392111] Code: c7 00 16 00 00 00 b8 ff ff ff ff c3 66 2e 0f 1f 84 00 00
> 00 00 00 f3 0f 1e fa 80 3d d5 ea 0e 00 00 74 13 b8 01 00 00 00 0f 05 <48> 3d
> 00 f0 ff ff 77 54 c3 0f 1f 00 55 48 89 e5 48 83 ec 20 48 89
> [ 191.393412] RSP: 002b:00007ffd95a23ab8 EFLAGS: 00000202 ORIG_RAX:
> 0000000000000001
> [ 191.393990] RAX: ffffffffffffffda RBX: 0000000000000005 RCX:
> 00007dbc4ff1c574
> [ 191.394594] RDX: 0000000000000005 RSI: 00005a6eeadb0ce0 RDI:
> 0000000000000001
> [ 191.395215] RBP: 00007ffd95a23ae0 R08: 00007dbc50003b20 R09:
> 0000000000000000
> [ 191.395805] R10: 0000000000000001 R11: 0000000000000202 R12:
> 0000000000000005
> [ 191.396404] R13: 00005a6eeadb0ce0 R14: 00007dbc500045c0 R15:
> 00007dbc50001ee0
> [ 191.396987] </TASK>
>
> To fix this, we explicitly set such mana debugfs variables to NULL after
> debugfs_remove() is called.
>
> Fixes: 6607c17c6c5e ("net: mana: Enable debugfs files for MANA device")
> Cc: sta...@vger.kernel.org
> Signed-off-by: Shradha Gupta <shradhagu...@linux.microsoft.com>
> Reviewed-by: Haiyang Zhang <haiya...@microsoft.com>
> ---
> drivers/net/ethernet/microsoft/mana/gdma_main.c | 11 ++++++++++-
> drivers/net/ethernet/microsoft/mana/mana_en.c | 10 ++++++----
> 2 files changed, 16 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> index c15a5ef4674e..f1966788c98e 100644
> --- a/drivers/net/ethernet/microsoft/mana/gdma_main.c
> +++ b/drivers/net/ethernet/microsoft/mana/gdma_main.c
> @@ -1579,6 +1579,7 @@ static int mana_gd_probe(struct pci_dev *pdev, const
> struct pci_device_id *ent)
> * adapter-MTU file and apc->mana_pci_debugfs folder.
> */
> debugfs_remove_recursive(gc->mana_pci_debugfs);
> + gc->mana_pci_debugfs = NULL;
> pci_iounmap(pdev, bar0_va);
> free_gc:
> pci_set_drvdata(pdev, NULL);
> @@ -1601,6 +1602,8 @@ static void mana_gd_remove(struct pci_dev *pdev)
>
> debugfs_remove_recursive(gc->mana_pci_debugfs);
>
> + gc->mana_pci_debugfs = NULL;
nitpick: I think the newline between "debug_remove_recursive" and
"gc->mana_pci_debugfs = NULL" is redundant (here and in similar places).
Logically, both lines are the code responsible for "mana_pci_debugfs" cleaning.
However, I don't think there's a need for sending v2 just because of
that.
> +
> pci_iounmap(pdev, gc->bar0_va);
>
> vfree(gc);
> @@ -1656,6 +1659,8 @@ static void mana_gd_shutdown(struct pci_dev *pdev)
>
> debugfs_remove_recursive(gc->mana_pci_debugfs);
>
> + gc->mana_pci_debugfs = NULL;
> +
> pci_disable_device(pdev);
> }
>
> @@ -1682,8 +1687,10 @@ static int __init mana_driver_init(void)
> mana_debugfs_root = debugfs_create_dir("mana", NULL);
>
> err = pci_register_driver(&mana_driver);
> - if (err)
> + if (err) {
> debugfs_remove(mana_debugfs_root);
> + mana_debugfs_root = NULL;
> + }
>
> return err;
> }
> @@ -1693,6 +1700,8 @@ static void __exit mana_driver_exit(void)
> pci_unregister_driver(&mana_driver);
>
> debugfs_remove(mana_debugfs_root);
> +
> + mana_debugfs_root = NULL;
> }
>
> module_init(mana_driver_init);
> diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c
> b/drivers/net/ethernet/microsoft/mana/mana_en.c
> index 2d826077d38c..9a8171f099b6 100644
> --- a/drivers/net/ethernet/microsoft/mana/mana_en.c
> +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c
> @@ -748,12 +748,11 @@ static const struct net_device_ops mana_devops = {
> static void mana_cleanup_port_context(struct mana_port_context *apc)
> {
> /*
> - * at this point all dir/files under the vport directory
> - * are already cleaned up.
> - * We are sure the apc->mana_port_debugfs remove will not
> - * cause any freed memory access issues
> + * make sure subsequent cleanup attempts don't end up removing already
> + * cleaned dentry pointer
> */
> debugfs_remove(apc->mana_port_debugfs);
> + apc->mana_port_debugfs = NULL;
> kfree(apc->rxqs);
> apc->rxqs = NULL;
> }
> @@ -1264,6 +1263,7 @@ static void mana_destroy_eq(struct mana_context *ac)
> return;
>
> debugfs_remove_recursive(ac->mana_eqs_debugfs);
> + ac->mana_eqs_debugfs = NULL;
>
> for (i = 0; i < gc->max_num_queues; i++) {
> eq = ac->eqs[i].eq;
> @@ -1926,6 +1926,7 @@ static void mana_destroy_txq(struct mana_port_context
> *apc)
>
> for (i = 0; i < apc->num_queues; i++) {
> debugfs_remove_recursive(apc->tx_qp[i].mana_tx_debugfs);
> + apc->tx_qp[i].mana_tx_debugfs = NULL;
>
> napi = &apc->tx_qp[i].tx_cq.napi;
> if (apc->tx_qp[i].txq.napi_initialized) {
> @@ -2113,6 +2114,7 @@ static void mana_destroy_rxq(struct mana_port_context
> *apc,
> return;
>
> debugfs_remove_recursive(rxq->mana_rx_debugfs);
> + rxq->mana_rx_debugfs = NULL;
>
> napi = &rxq->rx_cq.napi;
>
> --
> 2.34.1
>
Thanks,
Reviewed-by: Michal Kubiak <michal.kub...@intel.com>
