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 <[email protected]>
Signed-off-by: Andrea Parri (Microsoft) <[email protected]>
---
Changes since v1:
- Don't corrupt oldchannel if offer->child_relid is invalid
drivers/hv/channel_mgmt.c | 38 ++++++++++++++++++++++++--------------
drivers/hv/hyperv_vmbus.h | 2 +-
2 files changed, 25 insertions(+), 15 deletions(-)
diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c
index 68950a1e4b638..f91d476dfe381 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();
@@ -920,6 +926,8 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
oldchannel = find_primary_channel_by_offer(offer);
if (oldchannel != NULL) {
+ u32 relid = offer->child_relid;
+
/*
* We're resuming from hibernation: all the sub-channel and
* hv_sock channels we had before the hibernation should have
@@ -954,8 +962,10 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
atomic_dec(&vmbus_connection.offer_in_progress);
WARN_ON(oldchannel->offermsg.child_relid != INVALID_RELID);
+ if (WARN_ON(vmbus_connection.channels[relid] != NULL))
+ return;
/* Fix up the relid. */
- oldchannel->offermsg.child_relid = offer->child_relid;
+ oldchannel->offermsg.child_relid = relid;
offer_sz = sizeof(*offer);
if (memcmp(offer, &oldchannel->offermsg, offer_sz) != 0) {
@@ -967,7 +977,7 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr)
* reoffers the device upon resume.
*/
pr_debug("vmbus offer changed: relid=%d\n",
- offer->child_relid);
+ relid);
print_hex_dump_debug("Old vmbus offer: ",
DUMP_PREFIX_OFFSET, 16, 4,
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