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]

Date:   Wed, 8 Mar 2017 09:33:22 -0500
From:   Boris Ostrovsky <boris.ostrovsky@...cle.com>
To:     Stefano Stabellini <sstabellini@...nel.org>
Cc:     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


>
>>> +
>>> +		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?

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

-boris