From: Michal Kubiak <michal.kubiak@intel.com>
To: Shradha Gupta <shradhagupta@linux.microsoft.com>
Cc: <linux-hyperv@vger.kernel.org>, <netdev@vger.kernel.org>,
Long Li <longli@microsoft.com>,
Souradeep Chakrabarti <schakrabarti@linux.microsoft.com>,
Erick Archer <erick.archer@outlook.com>,
Leon Romanovsky <leon@kernel.org>,
Erni Sri Satya Vennela <ernis@linux.microsoft.com>,
Konstantin Taranov <kotaranov@microsoft.com>,
Peter Zijlstra <peterz@infradead.org>,
Maxim Levitsky <mlevitsk@redhat.com>,
Yury Norov <yury.norov@gmail.com>,
Michal Swiatkowski <michal.swiatkowski@linux.intel.com>,
Paolo Abeni <pabeni@redhat.com>,
"Jakub Kicinski" <kuba@kernel.org>,
Eric Dumazet <edumazet@google.com>,
Andrew Lunn <andrew+netdev@lunn.ch>,
Dexuan Cui <decui@microsoft.com>, Wei Liu <wei.liu@kernel.org>,
Haiyang Zhang <haiyangz@microsoft.com>,
"K. Y. Srinivasan" <kys@microsoft.com>,
Shradha Gupta <shradhagupta@microsoft.com>,
<stable@vger.kernel.org>
Subject: Re: [PATCH net] net: mana: cleanup mana struct after debugfs_remove()
Date: Tue, 11 Mar 2025 13:34:13 +0100 [thread overview]
Message-ID: <Z9AtxSilgYceOJCy@localhost.localdomain> (raw)
In-Reply-To: <1741688260-28922-1-git-send-email-shradhagupta@linux.microsoft.com>
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: stable@vger.kernel.org
> Signed-off-by: Shradha Gupta <shradhagupta@linux.microsoft.com>
> Reviewed-by: Haiyang Zhang <haiyangz@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.kubiak@intel.com>
next prev parent reply other threads:[~2025-03-11 12:35 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-03-11 10:17 [PATCH net] net: mana: cleanup mana struct after debugfs_remove() Shradha Gupta
2025-03-11 12:34 ` Michal Kubiak [this message]
2025-03-13 12:30 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=Z9AtxSilgYceOJCy@localhost.localdomain \
--to=michal.kubiak@intel.com \
--cc=andrew+netdev@lunn.ch \
--cc=decui@microsoft.com \
--cc=edumazet@google.com \
--cc=erick.archer@outlook.com \
--cc=ernis@linux.microsoft.com \
--cc=haiyangz@microsoft.com \
--cc=kotaranov@microsoft.com \
--cc=kuba@kernel.org \
--cc=kys@microsoft.com \
--cc=leon@kernel.org \
--cc=linux-hyperv@vger.kernel.org \
--cc=longli@microsoft.com \
--cc=michal.swiatkowski@linux.intel.com \
--cc=mlevitsk@redhat.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=peterz@infradead.org \
--cc=schakrabarti@linux.microsoft.com \
--cc=shradhagupta@linux.microsoft.com \
--cc=shradhagupta@microsoft.com \
--cc=stable@vger.kernel.org \
--cc=wei.liu@kernel.org \
--cc=yury.norov@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.