linux-kernel - Re: [PATCH v3 3/3] xen: optimize xenbus driver for multiple concurrent xenstore accesses

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, 7 Feb 2017 17:39:59 -0500
From:   Boris Ostrovsky <boris.ostrovsky@...cle.com>
To:     Juergen Gross <jgross@...e.com>, linux-kernel@...r.kernel.org,
        xen-devel@...ts.xenproject.org
Subject: Re: [PATCH v3 3/3] xen: optimize xenbus driver for multiple
 concurrent xenstore accesses

On 02/07/2017 12:51 PM, Boris Ostrovsky wrote:
> On 01/24/2017 11:23 AM, Juergen Gross wrote:
>> On 24/01/17 14:47, Boris Ostrovsky wrote:
>>> On 01/23/2017 01:59 PM, Boris Ostrovsky wrote:
>>>> On 01/23/2017 05:09 AM, Juergen Gross wrote:
>>>>> Handling of multiple concurrent Xenstore accesses through xenbus driver
>>>>> either from the kernel or user land is rather lame today: xenbus is
>>>>> capable to have one access active only at one point of time.
>>>>>
>>>>>
>>> This patch appears to break save/restore:
>> Hmm, tried multiple times, but I can't reproduce this issue.
>>
>> Anything special in the setup? I tried a 64 bit pv guest and did
>> "xl save".
>>
>> Do I have to run some load in parallel?
> Any luck reproducing this? I am still failing the test on dumpdata but I
> couldn't reproduce it on another system.


The problem appears to be xs_state_users being non-zero when we call
xs_suspend_enter().

>From what I understand this is caused by xs_request_exit() not
decrementing it when closing a transaction. This seems to be happening
when XS_TRANSACTION_END transaction returns XS_ERROR (I haven't traced
what causes this error but it doesn't appear to cause any visible harm).

Does the patch below make sense?

diff --git a/drivers/xen/xenbus/xenbus_xs.c b/drivers/xen/xenbus/xenbus_xs.c
index e62cb09..ffd5fac 100644
--- a/drivers/xen/xenbus/xenbus_xs.c
+++ b/drivers/xen/xenbus/xenbus_xs.c
@@ -140,7 +140,7 @@ void xs_request_exit(struct xb_req_data *req)
        spin_lock(&xs_state_lock);
        xs_state_users--;
        if ((req->type == XS_TRANSACTION_START && req->msg.type ==
XS_ERROR) ||
-           req->msg.type == XS_TRANSACTION_END)
+           req->type == XS_TRANSACTION_END)
                xs_state_users--;
        spin_unlock(&xs_state_lock);


I ran a few tests on dumpdata and they completed successfully. I'll keep
this for the overnight runs too, with a different Xen version.

-boris