> -----Original Message----- > From: Vitaly Kuznetsov [mailto:vkuznets@xxxxxxxxxx] > Sent: Wednesday, February 4, 2015 1:01 AM > To: KY Srinivasan; devel@xxxxxxxxxxxxxxxxxxxxxx > Cc: Haiyang Zhang; linux-kernel@xxxxxxxxxxxxxxx; Dexuan Cui; Jason Wang > Subject: [PATCH 3/4] Drivers: hv: vmbus: protect vmbus_get_outgoing_channel() > against channel removal > > list_for_each_safe() we have in vmbus_get_outgoing_channel() works, however, > we > are not protected against the channel being removed (e.g. after receiving > rescind > offer). Users of this function (storvsc_do_io() is the only one at this moment) > can get a link to an already freed channel. Make vmbus_get_outgoing_channel() > search holding primary->lock as child channels are not being freed unless they're > removed from parent's list. > > Signed-off-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx> > --- > drivers/hv/channel_mgmt.c | 10 +++++++--- > drivers/scsi/storvsc_drv.c | 2 ++ > 2 files changed, 9 insertions(+), 3 deletions(-) > > diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c > index fdccd16..af6243c 100644 > --- a/drivers/hv/channel_mgmt.c > +++ b/drivers/hv/channel_mgmt.c > @@ -881,18 +881,20 @@ cleanup: > */ > struct vmbus_channel *vmbus_get_outgoing_channel(struct vmbus_channel > *primary) > { > - struct list_head *cur, *tmp; > + struct list_head *cur; > int cur_cpu; > struct vmbus_channel *cur_channel; > struct vmbus_channel *outgoing_channel = primary; > int cpu_distance, new_cpu_distance; > + unsigned long flags; > > if (list_empty(&primary->sc_list)) > - return outgoing_channel; > + return vmbus_get_channel(outgoing_channel); > > cur_cpu = hv_context.vp_index[get_cpu()]; > put_cpu(); > - list_for_each_safe(cur, tmp, &primary->sc_list) { > + spin_lock_irqsave(&primary->lock, flags); hmm, we should avoid the locking here because it's a performance killer... How about adding vmbus_get/put_channel() in vmbus_open/close()? Thanks, -- Dexuan > + list_for_each(cur, &primary->sc_list) { > cur_channel = list_entry(cur, struct vmbus_channel, sc_list); > if (cur_channel->state != CHANNEL_OPENED_STATE) > continue; > @@ -913,6 +915,8 @@ struct vmbus_channel > *vmbus_get_outgoing_channel(struct vmbus_channel *primary) > > outgoing_channel = cur_channel; > } > + outgoing_channel = vmbus_get_channel(outgoing_channel); > + spin_unlock_irqrestore(&primary->lock, flags); > > return outgoing_channel; > } > diff --git a/drivers/scsi/storvsc_drv.c b/drivers/scsi/storvsc_drv.c > index 4cff0dd..3b9b851 100644 > --- a/drivers/scsi/storvsc_drv.c > +++ b/drivers/scsi/storvsc_drv.c > @@ -1370,6 +1370,8 @@ static int storvsc_do_io(struct hv_device *device, > > VMBUS_DATA_PACKET_FLAG_COMPLETION_REQUESTED); > } > > + vmbus_put_channel(outgoing_channel); > + > if (ret != 0) > return ret; > > -- > 1.9.3 _______________________________________________ devel mailing list devel@xxxxxxxxxxxxxxxxxxxxxx http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
