On Wed, Feb 24, 2021 at 07:50:09AM -0500, Sasha Levin wrote: > From: "Andrea Parri (Microsoft)" <parri.andrea@xxxxxxxxx> > > [ Upstream commit e4d221b42354b2e2ddb9187a806afb651eee2cda ] > > An erroneous or malicious host could send multiple rescind messages for > a same channel. In vmbus_onoffer_rescind(), the guest maps the channel > ID to obtain a pointer to the channel object and it eventually releases > such object and associated data. The host could time rescind messages > and lead to an use-after-free. Add a new flag to the channel structure > to make sure that only one instance of vmbus_onoffer_rescind() can get > the reference to the channel object. > > Reported-by: Juan Vazquez <juvazq@xxxxxxxxxxxxx> > Signed-off-by: Andrea Parri (Microsoft) <parri.andrea@xxxxxxxxx> > Reviewed-by: Michael Kelley <mikelley@xxxxxxxxxxxxx> > Link: https://lore.kernel.org/r/20201209070827.29335-6-parri.andrea@xxxxxxxxx > Signed-off-by: Wei Liu <wei.liu@xxxxxxxxxx> > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> Sasha - This patch is one of a group of patches where a Linux guest running on Hyper-V will start assuming that hypervisor behavior might be malicious, and guards against such behavior. Because this is a new assumption, these patches are more properly treated as new functionality rather than as bug fixes. So I would propose that we *not* bring such patches back to stable branches. Thanks, Andrea > --- > drivers/hv/channel_mgmt.c | 12 ++++++++++++ > include/linux/hyperv.h | 1 + > 2 files changed, 13 insertions(+) > > diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c > index 1d44bb635bb84..a9f58840f85dc 100644 > --- a/drivers/hv/channel_mgmt.c > +++ b/drivers/hv/channel_mgmt.c > @@ -1049,6 +1049,18 @@ static void vmbus_onoffer_rescind(struct vmbus_channel_message_header *hdr) > > mutex_lock(&vmbus_connection.channel_mutex); > channel = relid2channel(rescind->child_relid); > + if (channel != NULL) { > + /* > + * Guarantee that no other instance of vmbus_onoffer_rescind() > + * has got a reference to the channel object. Synchronize on > + * &vmbus_connection.channel_mutex. > + */ > + if (channel->rescind_ref) { > + mutex_unlock(&vmbus_connection.channel_mutex); > + return; > + } > + channel->rescind_ref = true; > + } > mutex_unlock(&vmbus_connection.channel_mutex); > > if (channel == NULL) { > diff --git a/include/linux/hyperv.h b/include/linux/hyperv.h > index 5ddb479c4d4cb..ef3573e99d989 100644 > --- a/include/linux/hyperv.h > +++ b/include/linux/hyperv.h > @@ -803,6 +803,7 @@ struct vmbus_channel { > u8 monitor_bit; > > bool rescind; /* got rescind msg */ > + bool rescind_ref; /* got rescind msg, got channel reference */ > struct completion rescind_event; > > u32 ringbuffer_gpadlhandle; > -- > 2.27.0 >
