linux-kernel - Re: [PATCH] Drivers: hv: util: on deinit, don't wait the release event, if we shouldn't

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 28 Feb 2017 15:21:11 +0100
From:   Vitaly Kuznetsov <vkuznets@...hat.com>
To:     Dexuan Cui <decui@...rosoft.com>
Cc:     Stephen Hemminger <sthemmin@...rosoft.com>,
        "gregkh\@linuxfoundation.org" <gregkh@...uxfoundation.org>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        "driverdev-devel\@linuxdriverproject.org" 
        <driverdev-devel@...uxdriverproject.org>,
        "Alex Ng \(LIS\)" <alexng@...rosoft.com>,
        "linux-kernel\@vger.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] Drivers: hv: util: on deinit, don't wait the release event, if we shouldn't

Vitaly Kuznetsov <vkuznets@...hat.com> writes:

> Dexuan Cui <decui@...rosoft.com> writes:
>
>>> From: devel [...] On Behalf Of Dexuan Cui
>>> > > --- 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. :-)
>>
>> BTW, IMO I found another potential issue:
>> In hvt_op_open -> hvt_reset -> kvp_on_reset(), I think we should call
>> init_completion() instead of complete()?
>>
>
> To me it looks like we can do better with something different from
> struct completion, I'll take a look later today.

Dexuan,

please take a look at the attached patch. After looking at the code
again it occured to me that it's going to be easier to move release wait
to the transport itself. Lightly tested.

Thanks!

-- 
  Vitaly


View attachment "0001-Drivers-hv-util-move-waiting-for-release-to-hv_utils.patch" of type "text/x-patch" (6426 bytes)