--- /dev/null
+From 3612a3f351c285ba07c33ac90ae13c549f07b869 Mon Sep 17 00:00:00 2001
+From: Sasha Levin <sashal@kernel.org>
+Date: Wed, 6 Nov 2024 07:42:47 -0800
+Subject: Drivers: hv: util: Avoid accessing a ringbuffer not initialized yet
+
+From: Michael Kelley <mhklinux@outlook.com>
+
+[ Upstream commit 07a756a49f4b4290b49ea46e089cbe6f79ff8d26 ]
+
+If the KVP (or VSS) daemon starts before the VMBus channel's ringbuffer is
+fully initialized, we can hit the panic below:
+
+hv_utils: Registering HyperV Utility Driver
+hv_vmbus: registering driver hv_utils
+...
+BUG: kernel NULL pointer dereference, address: 0000000000000000
+CPU: 44 UID: 0 PID: 2552 Comm: hv_kvp_daemon Tainted: G E 6.11.0-rc3+ #1
+RIP: 0010:hv_pkt_iter_first+0x12/0xd0
+Call Trace:
+...
+ vmbus_recvpacket
+ hv_kvp_onchannelcallback
+ vmbus_on_event
+ tasklet_action_common
+ tasklet_action
+ handle_softirqs
+ irq_exit_rcu
+ sysvec_hyperv_stimer0
+ </IRQ>
+ <TASK>
+ asm_sysvec_hyperv_stimer0
+...
+ kvp_register_done
+ hvt_op_read
+ vfs_read
+ ksys_read
+ __x64_sys_read
+
+This can happen because the KVP/VSS channel callback can be invoked
+even before the channel is fully opened:
+1) as soon as hv_kvp_init() -> hvutil_transport_init() creates
+/dev/vmbus/hv_kvp, the kvp daemon can open the device file immediately and
+register itself to the driver by writing a message KVP_OP_REGISTER1 to the
+file (which is handled by kvp_on_msg() ->kvp_handle_handshake()) and
+reading the file for the driver's response, which is handled by
+hvt_op_read(), which calls hvt->on_read(), i.e. kvp_register_done().
+
+2) the problem with kvp_register_done() is that it can cause the
+channel callback to be called even before the channel is fully opened,
+and when the channel callback is starting to run, util_probe()->
+vmbus_open() may have not initialized the ringbuffer yet, so the
+callback can hit the panic of NULL pointer dereference.
+
+To reproduce the panic consistently, we can add a "ssleep(10)" for KVP in
+__vmbus_open(), just before the first hv_ringbuffer_init(), and then we
+unload and reload the driver hv_utils, and run the daemon manually within
+the 10 seconds.
+
+Fix the panic by reordering the steps in util_probe() so the char dev
+entry used by the KVP or VSS daemon is not created until after
+vmbus_open() has completed. This reordering prevents the race condition
+from happening.
+
+Reported-by: Dexuan Cui <decui@microsoft.com>
+Fixes: e0fa3e5e7df6 ("Drivers: hv: utils: fix a race on userspace daemons registration")
+Cc: stable@vger.kernel.org
+Signed-off-by: Michael Kelley <mhklinux@outlook.com>
+Acked-by: Wei Liu <wei.liu@kernel.org>
+Link: https://lore.kernel.org/r/20241106154247.2271-3-mhklinux@outlook.com
+Signed-off-by: Wei Liu <wei.liu@kernel.org>
+Message-ID: <20241106154247.2271-3-mhklinux@outlook.com>
+Signed-off-by: Sasha Levin <sashal@kernel.org>
+---
+ drivers/hv/hv_kvp.c | 6 ++++++
+ drivers/hv/hv_snapshot.c | 6 ++++++
+ drivers/hv/hv_util.c | 9 +++++++++
+ drivers/hv/hyperv_vmbus.h | 2 ++
+ include/linux/hyperv.h | 1 +
+ 5 files changed, 24 insertions(+)
+
+diff --git a/drivers/hv/hv_kvp.c b/drivers/hv/hv_kvp.c
+index 5054d1105236..5b7dbf7b6f83 100644
+--- a/drivers/hv/hv_kvp.c
++++ b/drivers/hv/hv_kvp.c
+@@ -749,6 +749,12 @@ hv_kvp_init(struct hv_util_service *srv)
+ */
+ kvp_transaction.state = HVUTIL_DEVICE_INIT;
+
++ return 0;
++}
++
++int
++hv_kvp_init_transport(void)
++{
+ hvt = hvutil_transport_init(kvp_devname, CN_KVP_IDX, CN_KVP_VAL,
+ kvp_on_msg, kvp_on_reset);
+ if (!hvt)
+diff --git a/drivers/hv/hv_snapshot.c b/drivers/hv/hv_snapshot.c
+index 20ba95b75a94..9882e0b28a02 100644
+--- a/drivers/hv/hv_snapshot.c
++++ b/drivers/hv/hv_snapshot.c
+@@ -368,6 +368,12 @@ hv_vss_init(struct hv_util_service *srv)
+ */
+ vss_transaction.state = HVUTIL_DEVICE_INIT;
+
++ return 0;
++}
++
++int
++hv_vss_init_transport(void)
++{
+ hvt = hvutil_transport_init(vss_devname, CN_VSS_IDX, CN_VSS_VAL,
+ vss_on_msg, vss_on_reset);
+ if (!hvt) {
+diff --git a/drivers/hv/hv_util.c b/drivers/hv/hv_util.c
+index 48f9b1fbcbda..34bbccf2450e 100644
+--- a/drivers/hv/hv_util.c
++++ b/drivers/hv/hv_util.c
+@@ -98,12 +98,14 @@ static struct hv_util_service util_heartbeat = {
+ static struct hv_util_service util_kvp = {
+ .util_cb = hv_kvp_onchannelcallback,
+ .util_init = hv_kvp_init,
++ .util_init_transport = hv_kvp_init_transport,
+ .util_deinit = hv_kvp_deinit,
+ };
+
+ static struct hv_util_service util_vss = {
+ .util_cb = hv_vss_onchannelcallback,
+ .util_init = hv_vss_init,
++ .util_init_transport = hv_vss_init_transport,
+ .util_deinit = hv_vss_deinit,
+ };
+
+@@ -431,6 +433,13 @@ static int util_probe(struct hv_device *dev,
+ if (ret)
+ goto error;
+
++ if (srv->util_init_transport) {
++ ret = srv->util_init_transport();
++ if (ret) {
++ vmbus_close(dev->channel);
++ goto error;
++ }
++ }
+ return 0;
+
+ error:
+diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
+index 356382a340b2..0ddf482c6a53 100644
+--- a/drivers/hv/hyperv_vmbus.h
++++ b/drivers/hv/hyperv_vmbus.h
+@@ -353,10 +353,12 @@ void vmbus_on_event(unsigned long data);
+ void vmbus_on_msg_dpc(unsigned long data);
+
+ int hv_kvp_init(struct hv_util_service *srv);
++int hv_kvp_init_transport(void);
+ void hv_kvp_deinit(void);
+ void hv_kvp_onchannelcallback(void *context);
+
+ int hv_vss_init(struct hv_util_service *srv);
++int hv_vss_init_transport(void);
+ void hv_vss_deinit(void);
+ void hv_vss_onchannelcallback(void *context);
+
+diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h
+index 67d9b5a37460..1d9df8d22a06 100644
+--- a/include/linux/hyperv.h
++++ b/include/linux/hyperv.h
+@@ -1413,6 +1413,7 @@ struct hv_util_service {
+ void *channel;
+ void (*util_cb)(void *);
+ int (*util_init)(struct hv_util_service *);
++ int (*util_init_transport)(void);
+ void (*util_deinit)(void);
+ };
+
+--
+2.39.5
+
projects / thirdparty / kernel / stable-queue.git / commitdiff
