lkml.org 
[lkml]   [2019]   [Jan]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [PATCH] Drivers: hv: vmbus: Add mutex lock to channel show functions
On Tue, Jan 22, 2019 at 03:46:48AM +0000, Dexuan Cui wrote:
> > From: Kimberly Brown <kimbrownkd@gmail.com>
> > Sent: Monday, January 21, 2019 6:08 PM
> > Subject: [PATCH] Drivers: hv: vmbus: Add mutex lock to channel show functions
> >
> > The channel level "_show" functions are vulnerable to race conditions.
> > Add a mutex lock and unlock around the call to the channel level "_show"
> > functions in vmbus_chan_attr_show().
> >
> > This problem was discussed here:
> > https://lkml.org/lkml/2018/10/18/830
> >
> > --- a/drivers/hv/vmbus_drv.c
> > +++ b/drivers/hv/vmbus_drv.c
> > @@ -1414,6 +1414,7 @@ static ssize_t vmbus_chan_attr_show(struct kobject
> > *kobj,
> > = container_of(attr, struct vmbus_chan_attribute, attr);
> > const struct vmbus_channel *chan
> > = container_of(kobj, struct vmbus_channel, kobj);
> > + ssize_t ret;
> >
> > if (!attribute->show)
> > return -EIO;
> > @@ -1421,7 +1422,10 @@ static ssize_t vmbus_chan_attr_show(struct
> > kobject *kobj,
> > if (chan->state != CHANNEL_OPENED_STATE)
> > return -EINVAL;
> >
> > - return attribute->show(chan, buf);
> > + mutex_lock(&vmbus_connection.channel_mutex);
> > + ret = attribute->show(chan, buf);
> > + mutex_unlock(&vmbus_connection.channel_mutex);
> > + return ret;
> > }
>
> It looks this patch is already able to fix the original issue:
> 6712cc9c2211 ("vmbus: don't return values for uninitalized channels"),
> as chan->state can't be CHANNEL_OPENED_STATE when
> channel->ringbuffer_page is not set up yet, or has been freed.
>
> Thanks,
> -- Dexuan

I think that patch 6712cc9c2211 fixes the problem when the channel is
not set up yet, but it doesn't fix the problem when the channel is being
closed. The channel could be freed after the check that "chan->state" is
CHANNEL_OPENED_STATE, while the "attribute->show()" function is running.

Actually, there should be checks that "chan" is not null and that
"chan->state" is CHANNEL_OPENED_STATE within the locked section. I'll
need to fix that.

\
 
 \ /
  Last update: 2019-01-22 07:43    [W:0.117 / U:5.760 seconds]
©2003-2018 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site