From: "Guilherme G. Piccoli" <gpiccoli@igalia.com> To: linux-hyperv@vger.kernel.org Cc: kexec@lists.infradead.org, gpiccoli@igalia.com, kernel@gpiccoli.net, mikelley@microsoft.com, Tianyu.Lan@microsoft.com, kys@microsoft.com, haiyangz@microsoft.com, sthemmin@microsoft.com, wei.liu@kernel.org, decui@microsoft.com Subject: [PATCH] Drivers: hv: vmbus: Fix potential crash on module unload Date: Tue, 15 Mar 2022 17:35:35 -0300 [thread overview] Message-ID: <20220315203535.682306-1-gpiccoli@igalia.com> (raw) The vmbus driver relies on the panic notifier infrastructure to perform some operations when a panic event is detected. Since vmbus can be built as module, it is required that the driver handles both registering and unregistering such panic notifier callback. After commit 74347a99e73a ("x86/Hyper-V: Unload vmbus channel in hv panic callback") though, the panic notifier registration is done unconditionally in the module initialization routine whereas the unregistering procedure is conditionally guarded and executes only if HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE capability is set. This patch fixes that by unconditionally unregistering the panic notifier in the module's exit routine as well. Fixes: 74347a99e73a ("x86/Hyper-V: Unload vmbus channel in hv panic callback") Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com> --- Hi folks, thanks in advance for any reviews! This was build-tested with Debian config, on 5.17-rc7. This patch is a result of code analysis - I didn't experience this issue but seems a valid/feasible case. Also, this is part of an ongoing effort of clearing/refactoring the panic notifiers, more will be done soon, but I prefer to send the simple bug fixes quickly, or else it might take a while since the next steps are more complex and subject to many iterations I expect. Cheers, Guilherme drivers/hv/vmbus_drv.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 12a2b37e87f3..12585324cc4a 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -2780,10 +2780,15 @@ static void __exit vmbus_exit(void) if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) { kmsg_dump_unregister(&hv_kmsg_dumper); unregister_die_notifier(&hyperv_die_block); - atomic_notifier_chain_unregister(&panic_notifier_list, - &hyperv_panic_block); } + /* + * The panic notifier is always registered, hence we should + * also unconditionally unregister it here as well. + */ + atomic_notifier_chain_unregister(&panic_notifier_list, + &hyperv_panic_block); + free_page((unsigned long)hv_panic_page); unregister_sysctl_table(hv_ctl_table_hdr); hv_ctl_table_hdr = NULL; -- 2.35.1
WARNING: multiple messages have this Message-ID (diff)
From: Guilherme G. Piccoli <gpiccoli@igalia.com> To: kexec@lists.infradead.org Subject: [PATCH] Drivers: hv: vmbus: Fix potential crash on module unload Date: Tue, 15 Mar 2022 17:35:35 -0300 [thread overview] Message-ID: <20220315203535.682306-1-gpiccoli@igalia.com> (raw) The vmbus driver relies on the panic notifier infrastructure to perform some operations when a panic event is detected. Since vmbus can be built as module, it is required that the driver handles both registering and unregistering such panic notifier callback. After commit 74347a99e73a ("x86/Hyper-V: Unload vmbus channel in hv panic callback") though, the panic notifier registration is done unconditionally in the module initialization routine whereas the unregistering procedure is conditionally guarded and executes only if HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE capability is set. This patch fixes that by unconditionally unregistering the panic notifier in the module's exit routine as well. Fixes: 74347a99e73a ("x86/Hyper-V: Unload vmbus channel in hv panic callback") Signed-off-by: Guilherme G. Piccoli <gpiccoli@igalia.com> --- Hi folks, thanks in advance for any reviews! This was build-tested with Debian config, on 5.17-rc7. This patch is a result of code analysis - I didn't experience this issue but seems a valid/feasible case. Also, this is part of an ongoing effort of clearing/refactoring the panic notifiers, more will be done soon, but I prefer to send the simple bug fixes quickly, or else it might take a while since the next steps are more complex and subject to many iterations I expect. Cheers, Guilherme drivers/hv/vmbus_drv.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c index 12a2b37e87f3..12585324cc4a 100644 --- a/drivers/hv/vmbus_drv.c +++ b/drivers/hv/vmbus_drv.c @@ -2780,10 +2780,15 @@ static void __exit vmbus_exit(void) if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) { kmsg_dump_unregister(&hv_kmsg_dumper); unregister_die_notifier(&hyperv_die_block); - atomic_notifier_chain_unregister(&panic_notifier_list, - &hyperv_panic_block); } + /* + * The panic notifier is always registered, hence we should + * also unconditionally unregister it here as well. + */ + atomic_notifier_chain_unregister(&panic_notifier_list, + &hyperv_panic_block); + free_page((unsigned long)hv_panic_page); unregister_sysctl_table(hv_ctl_table_hdr); hv_ctl_table_hdr = NULL; -- 2.35.1
next reply other threads:[~2022-03-15 21:17 UTC|newest] Thread overview: 6+ messages / expand[flat|nested] mbox.gz Atom feed top 2022-03-15 20:35 Guilherme G. Piccoli [this message] 2022-03-15 20:35 ` [PATCH] Drivers: hv: vmbus: Fix potential crash on module unload Guilherme G. Piccoli 2022-03-19 15:30 ` Michael Kelley (LINUX) 2022-03-19 15:30 ` Michael Kelley 2022-03-29 12:05 ` Wei Liu 2022-03-29 12:05 ` Wei Liu
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=20220315203535.682306-1-gpiccoli@igalia.com \ --to=gpiccoli@igalia.com \ --cc=Tianyu.Lan@microsoft.com \ --cc=decui@microsoft.com \ --cc=haiyangz@microsoft.com \ --cc=kernel@gpiccoli.net \ --cc=kexec@lists.infradead.org \ --cc=kys@microsoft.com \ --cc=linux-hyperv@vger.kernel.org \ --cc=mikelley@microsoft.com \ --cc=sthemmin@microsoft.com \ --cc=wei.liu@kernel.org \ /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: linkBe 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.