linux-kernel - [RFC PATCH] Hyper-V: Initialize crash reporting before vmbus

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <YqtAyitIGRAHL7V0@czspare1-lap.sysgo.cz>
Date:   Thu, 16 Jun 2022 16:40:10 +0200
From:   Vit Kabele <vit.kabele@...go.com>
To:     linux-hyperv@...r.kernel.org
Cc:     mikelley@...rosoft.com, linux-kernel@...r.kernel.org,
        kys@...rosoft.com
Subject: [RFC PATCH] Hyper-V: Initialize crash reporting before vmbus

The Hyper-V crash reporting feature is initialized after a successful
vmbus setup. The reporting feature however does not require vmbus at all
and Windows guests can indeed use the reporting capabilities even with
the minimal Hyper-V implementation (as described in the Minimal
Requirements document).

Reorder the initialization callbacks so that the crash reporting
callbacks are registered before the vmbus initialization starts.

Nevertheless, I am not sure about following:

1/ The vmbus_initiate_unload function is called within the panic handler
even when the vmbus initialization does not finish (there might be no
vmbus at all). This should probably not be problem because the vmbus
unload function always checks for current connection state and does
nothing when this is "DISCONNECTED". For better readability, it might be
better to add separate panic notifier for vmbus and crash reporting.

2/ Wouldn't it be better to extract the whole reporting capability out
of the vmbus module, so that it stays present in the kernel even when
the vmbus module is possibly unloaded?

Signed-off-by: Vit Kabele <vit.kabele@...go.com>

---
 drivers/hv/vmbus_drv.c | 77 +++++++++++++++++++++++-------------------
 1 file changed, 42 insertions(+), 35 deletions(-)

diff --git a/drivers/hv/vmbus_drv.c b/drivers/hv/vmbus_drv.c
index 714d549b7b46..97873f03aa7a 100644
--- a/drivers/hv/vmbus_drv.c
+++ b/drivers/hv/vmbus_drv.c
@@ -1509,41 +1509,6 @@ static int vmbus_bus_init(void)
 	if (hv_is_isolation_supported())
 		sysctl_record_panic_msg = 0;
 
-	/*
-	 * Only register if the crash MSRs are available
-	 */
-	if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
-		u64 hyperv_crash_ctl;
-		/*
-		 * Panic message recording (sysctl_record_panic_msg)
-		 * is enabled by default in non-isolated guests and
-		 * disabled by default in isolated guests; the panic
-		 * message recording won't be available in isolated
-		 * guests should the following registration fail.
-		 */
-		hv_ctl_table_hdr = register_sysctl_table(hv_root_table);
-		if (!hv_ctl_table_hdr)
-			pr_err("Hyper-V: sysctl table register error");
-
-		/*
-		 * Register for panic kmsg callback only if the right
-		 * capability is supported by the hypervisor.
-		 */
-		hyperv_crash_ctl = hv_get_register(HV_REGISTER_CRASH_CTL);
-		if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG)
-			hv_kmsg_dump_register();
-
-		register_die_notifier(&hyperv_die_block);
-	}
-
-	/*
-	 * Always register the panic notifier because we need to unload
-	 * the VMbus channel connection to prevent any VMbus
-	 * activity after the VM panics.
-	 */
-	atomic_notifier_chain_register(&panic_notifier_list,
-			       &hyperv_panic_block);
-
 	vmbus_request_offers();
 
 	return 0;
@@ -2675,6 +2640,46 @@ static struct syscore_ops hv_synic_syscore_ops = {
 	.resume = hv_synic_resume,
 };
 
+static void __init crash_reporting_init(void)
+{
+	/*
+	 * Only register if the crash MSRs are available
+	 */
+	if (ms_hyperv.misc_features & HV_FEATURE_GUEST_CRASH_MSR_AVAILABLE) {
+		u64 hyperv_crash_ctl;
+		/*
+		 * Panic message recording (sysctl_record_panic_msg)
+		 * is enabled by default in non-isolated guests and
+		 * disabled by default in isolated guests; the panic
+		 * message recording won't be available in isolated
+		 * guests should the following registration fail.
+		 */
+		hv_ctl_table_hdr = register_sysctl_table(hv_root_table);
+		if (!hv_ctl_table_hdr)
+			pr_err("Hyper-V: sysctl table register error");
+
+		/*
+		 * Register for panic kmsg callback only if the right
+		 * capability is supported by the hypervisor.
+		 */
+		hyperv_crash_ctl = hv_get_register(HV_REGISTER_CRASH_CTL);
+		if (hyperv_crash_ctl & HV_CRASH_CTL_CRASH_NOTIFY_MSG)
+			hv_kmsg_dump_register();
+
+		register_die_notifier(&hyperv_die_block);
+	}
+
+	/*
+	 * Always register the panic notifier because we need to unload
+	 * the VMbus channel connection to prevent any VMbus
+	 * activity after the VM panics.
+	 */
+	atomic_notifier_chain_register(&panic_notifier_list,
+			       &hyperv_panic_block);
+
+
+}
+
 static int __init hv_acpi_init(void)
 {
 	int ret, t;
@@ -2687,6 +2692,8 @@ static int __init hv_acpi_init(void)
 
 	init_completion(&probe_event);
 
+	crash_reporting_init();
+
 	/*
 	 * Get ACPI resources first.
 	 */
-- 
2.30.2