linux-kernel - Re: [PATCH 6/7] xen/9pfs: receive responses

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]

Message-ID: <alpine.DEB.2.10.1703081125310.8160@sstabellini-ThinkPad-X260>
Date:   Wed, 8 Mar 2017 11:26:03 -0800 (PST)
From:   Stefano Stabellini <sstabellini@...nel.org>
To:     Boris Ostrovsky <boris.ostrovsky@...cle.com>
cc:     Stefano Stabellini <sstabellini@...nel.org>,
        xen-devel@...ts.xenproject.org, linux-kernel@...r.kernel.org,
        Stefano Stabellini <stefano@...reto.com>, jgross@...e.com,
        Eric Van Hensbergen <ericvh@...il.com>,
        Ron Minnich <rminnich@...dia.gov>,
        Latchesar Ionkov <lucho@...kov.net>,
        v9fs-developer@...ts.sourceforge.net
Subject: Re: [PATCH 6/7] xen/9pfs: receive responses

On Wed, 8 Mar 2017, Boris Ostrovsky wrote:
> >>> +
> >>> +		if (xen_9pfs_queued(prod, cons, XEN_9PFS_RING_SIZE) < sizeof(h)) {
> >>> +			notify_remote_via_irq(ring->irq);
> >>> +			return;
> >>> +		}
> >>> +
> >>> +		masked_prod = xen_9pfs_mask(prod, XEN_9PFS_RING_SIZE);
> >>> +		masked_cons = xen_9pfs_mask(cons, XEN_9PFS_RING_SIZE);
> >>> +
> >>> +		xen_9pfs_read_packet(ring->ring.in,
> >>> +				masked_prod, &masked_cons,
> >>> +				XEN_9PFS_RING_SIZE, &h, sizeof(h));
> >>> +
> >>> +		req = p9_tag_lookup(priv->client, h.tag);
> >>> +		if (!req || req->status != REQ_STATUS_SENT) {
> >>> +			dev_warn(&priv->dev->dev, "Wrong req tag=%x\n", h.tag);
> >>> +			cons += h.size;
> >>> +			mb();
> >>> +			ring->intf->in_cons = cons;
> >>> +			continue;
> >>
> >> I don't know what xen_9pfs_read_packet() does so perhaps it's done there
> >> but shouldn't the pointers be updated regardless of the 'if' condition?
> > This is the error path - the index is increased immediately. In the
> > non-error case, we do that right after the next read_packet call, few
> > lines below.
> >
> >
> >>> +		}
> >>> +
> >>> +		memcpy(req->rc, &h, sizeof(h));
> >>> +		req->rc->offset = 0;
> >>> +
> >>> +		masked_cons = xen_9pfs_mask(cons, XEN_9PFS_RING_SIZE);
> >>> +		xen_9pfs_read_packet(ring->ring.in,
> >>> +				masked_prod, &masked_cons,
> >>> +				XEN_9PFS_RING_SIZE, req->rc->sdata, h.size);
> >>> +
> >>> +		mb();
> >>> +		cons += h.size;
> >>> +		ring->intf->in_cons = cons;
> >                    Here ^
> >
> 
> 
> So the second read is reading again from the same pointer in the ring,
> but this time it gets the whole packet, including the header. The first
> read was just poking at the header. Right?

That's right. First we read the header, to know how much data to read.


> If that's correct, can you add a comment somewhere? (unless this is
> obvious to everyone else but me.)

Sure.