> From: Vitaly Kuznetsov [mailto:vkuzn...@redhat.com]
> > void hv_fcopy_deinit(void)
> > {
> > + bool wait = hvt->dev_opened;
> > +
> > fcopy_transaction.state = HVUTIL_DEVICE_DYING;
> > cancel_delayed_work_sync(&fcopy_timeout_work);
> > hvutil_transport_destroy(hvt);
> > - wait_for_completion(&release_event);
> > + if (wait)
> > + wait_for_completion(&release_event);
>
> This is racy I think. We need to prevent openning the device first and
> then query its state:
>
> bool wait;
>
> fcopy_transaction.state = HVUTIL_DEVICE_DYING;
> /* make sure state is set */
> mb();
> wait = hvt->dev_opened;
> cancel_delayed_work_sync(&fcopy_timeout_work);
> hvutil_transport_destroy(hvt);
> if (wait)
> wait_for_completion(&release_event);
>
> otherwise someone could open the device before we manage to update its
> state.
I agree.
> > @@ -182,6 +183,7 @@ static int hvt_op_release(struct inode *inode,
> struct file *file)
> > * connects back.
> > */
> > hvt_reset(hvt);
> > + hvt->dev_opened = false;
> > mutex_unlock(&hvt->lock);
> >
>
> Not sure but it seems this may also be racy (what if we query the state
> just before we reset it?).
Yeah, I agree.
> > if (mode_old == HVUTIL_TRANSPORT_DESTROY)
> > diff --git a/drivers/hv/hv_utils_transport.h
> b/drivers/hv/hv_utils_transport.h
> > index d98f522..9871283 100644
> > --- a/drivers/hv/hv_utils_transport.h
> > +++ b/drivers/hv/hv_utils_transport.h
> > @@ -32,6 +32,7 @@ struct hvutil_transport {
> > int mode; /* hvutil_transport_mode */
> > struct file_operations fops; /* file operations */
> > struct miscdevice mdev; /* misc device */
> > + bool dev_opened; /* Is the device opened? */
> > struct cb_id cn_id; /* CN_*_IDX/CN_*_VAL */
> > struct list_head list; /* hvt_list */
> > int (*on_msg)(void *, int); /* callback on new user message */
>
> I think we can get away without introducing this new flag, e.g. if we
> replace release_event with an atomic which will hold the state
> (open/closed). This will also elimenate possible races above. I can try
> prototyping a patch if you want me to.
> --
> Vitaly
Thanks for offering the help! Please do. :-)
Thanks,
-- Dexuan
_______________________________________________
devel mailing list
de...@linuxdriverproject.org
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel
