[PATCH 6/6] Drivers: hv: vmbus: Do not allow overwriting vmbus_connection.channels[]
From: "Andrea Parri (Microsoft)" <parri.andrea@gmail.com>
Date: 2020-11-18 14:38:07
Also in:
lkml
Subsystem:
hyper-v/azure core and drivers, the rest · Maintainers:
"K. Y. Srinivasan", Haiyang Zhang, Wei Liu, Dexuan Cui, Long Li, Linus Torvalds
Currently, vmbus_onoffer() and vmbus_process_offer() are not validating whether a given entry in the vmbus_connection.channels[] array is empty before filling the entry with a call of vmbus_channel_map_relid(). An erroneous or malicious host could rely on this to leak channel objects. Do not allow overwriting an entry vmbus_connection.channels[]. Reported-by: Juan Vazquez <redacted> Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@gmail.com> --- drivers/hv/channel_mgmt.c | 30 ++++++++++++++++++------------ drivers/hv/hyperv_vmbus.h | 2 +- 2 files changed, 19 insertions(+), 13 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 68950a1e4b638..841c0d4e101bd 100644
--- a/drivers/hv/channel_mgmt.c
+++ b/drivers/hv/channel_mgmt.c@@ -354,10 +354,12 @@ static void free_channel(struct vmbus_channel *channel) kobject_put(&channel->kobj); } -void vmbus_channel_map_relid(struct vmbus_channel *channel) +int vmbus_channel_map_relid(struct vmbus_channel *channel) { - if (WARN_ON(channel->offermsg.child_relid >= MAX_CHANNEL_RELIDS)) - return; + u32 relid = channel->offermsg.child_relid; + + if (WARN_ON(relid >= MAX_CHANNEL_RELIDS || vmbus_connection.channels[relid] != NULL)) + return -EINVAL; /* * The mapping of the channel's relid is visible from the CPUs that * execute vmbus_chan_sched() by the time that vmbus_chan_sched() will
@@ -383,18 +385,17 @@ void vmbus_channel_map_relid(struct vmbus_channel *channel) * of the VMBus driver and vmbus_chan_sched() can not run before * vmbus_bus_resume() has completed execution (cf. resume_noirq). */ - smp_store_mb( - vmbus_connection.channels[channel->offermsg.child_relid], - channel); + smp_store_mb(vmbus_connection.channels[relid], channel); + return 0; } void vmbus_channel_unmap_relid(struct vmbus_channel *channel) { - if (WARN_ON(channel->offermsg.child_relid >= MAX_CHANNEL_RELIDS)) + u32 relid = channel->offermsg.child_relid; + + if (WARN_ON(relid >= MAX_CHANNEL_RELIDS)) return; - WRITE_ONCE( - vmbus_connection.channels[channel->offermsg.child_relid], - NULL); + WRITE_ONCE(vmbus_connection.channels[relid], NULL); } static void vmbus_release_relid(u32 relid)
@@ -601,6 +602,12 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel) */ atomic_dec(&vmbus_connection.offer_in_progress); + if (vmbus_channel_map_relid(newchannel)) { + mutex_unlock(&vmbus_connection.channel_mutex); + kfree(newchannel); + return; + } + list_for_each_entry(channel, &vmbus_connection.chn_list, listentry) { if (guid_equal(&channel->offermsg.offer.if_type, &newchannel->offermsg.offer.if_type) &&
@@ -619,6 +626,7 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel) * Check to see if this is a valid sub-channel. */ if (newchannel->offermsg.offer.sub_channel_index == 0) { + vmbus_channel_unmap_relid(newchannel); mutex_unlock(&vmbus_connection.channel_mutex); /* * Don't call free_channel(), because newchannel->kobj
@@ -635,8 +643,6 @@ static void vmbus_process_offer(struct vmbus_channel *newchannel) list_add_tail(&newchannel->sc_list, &channel->sc_list); } - vmbus_channel_map_relid(newchannel); - mutex_unlock(&vmbus_connection.channel_mutex); cpus_read_unlock();
diff --git a/drivers/hv/hyperv_vmbus.h b/drivers/hv/hyperv_vmbus.h
index e2064bf2b557d..89d7b95b3bdad 100644
--- a/drivers/hv/hyperv_vmbus.h
+++ b/drivers/hv/hyperv_vmbus.h@@ -338,7 +338,7 @@ int vmbus_add_channel_kobj(struct hv_device *device_obj, void vmbus_remove_channel_attr_group(struct vmbus_channel *channel); -void vmbus_channel_map_relid(struct vmbus_channel *channel); +int vmbus_channel_map_relid(struct vmbus_channel *channel); void vmbus_channel_unmap_relid(struct vmbus_channel *channel); struct vmbus_channel *relid2channel(u32 relid);
--
2.25.1