linux-kernel - Re: [PATCH] xen: fix frontend driver disconnected from xenbus on removal

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:   Thu, 1 Feb 2018 16:09:25 -0500
From:   Boris Ostrovsky <boris.ostrovsky@...cle.com>
To:     Oleksandr Andrushchenko <andr2000@...il.com>,
        xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org
Cc:     jgross@...e.com, otubo@...hat.com,
        Oleksandr Andrushchenko <oleksandr_andrushchenko@...m.com>
Subject: Re: [PATCH] xen: fix frontend driver disconnected from xenbus on
 removal

On 02/01/2018 03:24 PM, Oleksandr Andrushchenko wrote:
>
>
> On 02/01/2018 10:08 PM, Boris Ostrovsky wrote:
>> On 02/01/2018 03:57 AM, Oleksandr Andrushchenko wrote:
>>> From: Oleksandr Andrushchenko <oleksandr_andrushchenko@...m.com>
>>>
>>> Current xenbus frontend driver removal flow first disconnects
>>> the driver from xenbus and then calls driver's remove callback.
>>> This makes it impossible for the driver to listen to backend's
>>> state changes and synchronize the removal procedure.
>>>
>>> Fix this by removing other end XenBus watches after the
>>> driver's remove callback is called.
>>>
>>> Signed-off-by: Oleksandr Andrushchenko
>>> <oleksandr_andrushchenko@...m.com>
>>> ---
>>>   drivers/xen/xenbus/xenbus_probe.c | 4 ++--
>>>   1 file changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/drivers/xen/xenbus/xenbus_probe.c
>>> b/drivers/xen/xenbus/xenbus_probe.c
>>> index 74888cacd0b0..9c63cd3f416b 100644
>>> --- a/drivers/xen/xenbus/xenbus_probe.c
>>> +++ b/drivers/xen/xenbus/xenbus_probe.c
>>> @@ -258,11 +258,11 @@ int xenbus_dev_remove(struct device *_dev)
>>>         DPRINTK("%s", dev->nodename);
>>>   -    free_otherend_watch(dev);
>>> -
>>>       if (drv->remove)
>>>           drv->remove(dev);
>>
>> Is it possible for the watch to fire here?
> Indeed. Yes, It is possible, so we have to somehow protect the removed
> driver from being called, e.g. the driver cleans up in its .remove,
> but watch may still trigger .otherend_changed callback.
> Is this what you mean?

(-David who is not at Citrix anymore)

Exactly.

That's why otherend cleanup is split into free_otherend_watch() and
free_otherend_details().


> If so, do you have something neat on your mind how to solve this?

Not necessarily "neat" but perhaps you can use
xenbus_read_otherend_details() in both front and back ends. After all,
IIUIC you are doing something synchronously so you don't really need a
watch.

-boris

>
>> -boris
>>
>>>   +    free_otherend_watch(dev);
>>> +
>>>       free_otherend_details(dev);
>>>         xenbus_switch_state(dev, XenbusStateClosed);
> Thank you,
> Oleksandr