This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
Home Products Support Community News


Re: [Xen-devel] A bug in Xenbus driver

To: "Jun Zhu (Intern)" <Jun.Zhu@xxxxxxxxxx>
Subject: Re: [Xen-devel] A bug in Xenbus driver
From: Jeremy Fitzhardinge <jeremy@xxxxxxxx>
Date: Wed, 25 Aug 2010 12:11:27 -0700
Cc: "xen-devel@xxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxx>
Delivery-date: Wed, 25 Aug 2010 12:12:24 -0700
Envelope-to: www-data@xxxxxxxxxxxxxxxxxxx
In-reply-to: <433DDF91DFB08148BAD3FDB6FDDA314C9F35F3BB47@xxxxxxxxxxxxxxxxxxxxxxxxx>
List-help: <mailto:xen-devel-request@lists.xensource.com?subject=help>
List-id: Xen developer discussion <xen-devel.lists.xensource.com>
List-post: <mailto:xen-devel@lists.xensource.com>
List-subscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=subscribe>
List-unsubscribe: <http://lists.xensource.com/mailman/listinfo/xen-devel>, <mailto:xen-devel-request@lists.xensource.com?subject=unsubscribe>
References: <433DDF91DFB08148BAD3FDB6FDDA314C9F35F3BB47@xxxxxxxxxxxxxxxxxxxxxxxxx>
Sender: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx
User-agent: Mozilla/5.0 (X11; U; Linux x86_64; en-US; rv: Gecko/20100806 Fedora/3.1.2-1.fc13 Lightning/1.0b2pre Thunderbird/3.1.2
 On 08/25/2010 11:57 AM, Jun Zhu (Intern) wrote:
> Hi all,
> I think this is a serious bug, existing in the pvops (also existing 
> in linux 2.6.35);
> In the xenbus driver (drivers/xen/xenfs/xenbus.c), the function of 
> xenbus_file_read has a section of source code like this:
>               if (ret != sz) {
>                   if (i == 0)
>                                 i = -EFAULT;
>                         goto out;
>                 }
>               /* Clear out buffer if it has been consumed */
>               if (rb->cons == rb->len) {
>                       list_del(&rb->list);
>                       kfree(rb);
>                       if (list_empty(&u->read_buffers))
>                               break;
>                       rb = list_entry(u->read_buffers.next,
>                                       struct read_buffer, list);
>               }
> It should be like this:
> //              if (ret != sz) {
>                 if (ret != 0) {
>                         if (i == 0)
>                                 i = -EFAULT;
>                         goto out;
>                 }
> This bug makes the read_buffer not be cleared most of the time. If the 
> xenstore client uses PTHREAD to create a thread to receive reply message, the 
> problem will incur. The new thread can not read what it wants to read, since 
> the list is not empty.
> I found this problem from the xenstore client xs_watch function. xs_watch 
> creates the new thread on demand. So I recommend that in the function of 
> read_message(xen/tools/xenstore/xs.c), if using thread to receive message, in 
> the case of read fault, it should signal to the listener and print out the 
> error. 

I don't follow your description of the problem.  The full context of the
code in question is this:

                ret = copy_to_user(ubuf + i, &rb->msg[rb->cons], sz);

                i += sz - ret;
                rb->cons += sz - ret;

                if (ret != sz) {
                        if (i == 0)
                                i = -EFAULT;
                        goto out;

copy_to_user returns the number of uncopied bytes, so if ret != sz, then
some part of the data was not copied due to a memory fault problem.

Changing the test to "ret != 0" means that it will return a partial
result (at the end of a page, for example) without reporting an error,
and discarding the uncopied data.

If you're hitting the "ret != sz" case often, then I think there's some
problem with the memory you're passing into read() (either the buffer
itself, or the size argument).


Xen-devel mailing list

<Prev in Thread] Current Thread [Next in Thread>