[PATCH net-next] macvtap: advance iov iterator when needed in macvtap_put_user()

[PATCH net-next] macvtap: advance iov iterator when needed in macvtap_put_user()

[Jason Wang]

When mergeable buffer is used, vnet_hdr_sz is greater than sizeof struct
virtio_net_hdr. So we need advance the iov iterators in this case.

Fixes 6c36d2e26cda1ad3e2c4b90dd843825fc62fe5b4 ("macvtap: Use iovec iterators")
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Signed-off-by: Jason Wang <jasowang@redhat.com>
---
 drivers/net/macvtap.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index cea99d4..42a80d3 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -797,6 +797,8 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
 		if (copy_to_iter(&vnet_hdr, sizeof(vnet_hdr), iter) !=
 		    sizeof(vnet_hdr))
 			return -EFAULT;
+
+		iov_iter_advance(iter, vnet_hdr_len - sizeof(vnet_hdr));
 	}
 	total = vnet_hdr_len;
 	total += skb->len;
-- 
1.9.1

Re: [PATCH net-next] macvtap: advance iov iterator when needed in macvtap_put_user()

[Herbert Xu]

On Thu, Nov 20, 2014 at 04:31:05PM +0800, Jason Wang wrote:
> When mergeable buffer is used, vnet_hdr_sz is greater than sizeof struct
> virtio_net_hdr. So we need advance the iov iterators in this case.
> 
> Fixes 6c36d2e26cda1ad3e2c4b90dd843825fc62fe5b4 ("macvtap: Use iovec iterators")
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks,
-- 
Email: Herbert Xu <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCH net-next] macvtap: advance iov iterator when needed in macvtap_put_user()

[Al Viro]

On Thu, Nov 20, 2014 at 04:34:26PM +0800, Herbert Xu wrote:
> On Thu, Nov 20, 2014 at 04:31:05PM +0800, Jason Wang wrote:
> > When mergeable buffer is used, vnet_hdr_sz is greater than sizeof struct
> > virtio_net_hdr. So we need advance the iov iterators in this case.
> > 
> > Fixes 6c36d2e26cda1ad3e2c4b90dd843825fc62fe5b4 ("macvtap: Use iovec iterators")
> > Cc: Herbert Xu <herbert@gondor.apana.org.au>
> > Signed-off-by: Jason Wang <jasowang@redhat.com>
> 
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

I'm not sure it's a good behaviour, actually - should we just leave an
unmodified gap in userland buffer there, with whatever garbage it might
have contained?

I do realize that this is what we used to do, but it might end up a source
of hard to debug userland bugs...  Perhaps that iov_iter_advance(iter, size)
would better be replaced with iov_iter_zero(size, iter)?

Re: [PATCH net-next] macvtap: advance iov iterator when needed in macvtap_put_user()

[Jason Wang]

On 11/20/2014 04:41 PM, Al Viro wrote:
> On Thu, Nov 20, 2014 at 04:34:26PM +0800, Herbert Xu wrote:
>> On Thu, Nov 20, 2014 at 04:31:05PM +0800, Jason Wang wrote:
>>> When mergeable buffer is used, vnet_hdr_sz is greater than sizeof struct
>>> virtio_net_hdr. So we need advance the iov iterators in this case.
>>>
>>> Fixes 6c36d2e26cda1ad3e2c4b90dd843825fc62fe5b4 ("macvtap: Use iovec iterators")
>>> Cc: Herbert Xu <herbert@gondor.apana.org.au>
>>> Signed-off-by: Jason Wang <jasowang@redhat.com>
>> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
> I'm not sure it's a good behaviour, actually - should we just leave an
> unmodified gap in userland buffer there, with whatever garbage it might
> have contained?

Anyway it's the users(qemu or vhost_net) responsibility to fill the gap
here.
> I do realize that this is what we used to do, but it might end up a source
> of hard to debug userland bugs...  Perhaps that iov_iter_advance(iter, size)
> would better be replaced with iov_iter_zero(size, iter)?

Not sure, macvtap or tun behave like this in the past. It's easy to dump
the header in guest for debugging. Zero the gap probably only help the
case that the gap was forgot to be filled which looks rare.

Re: [PATCH net-next] macvtap: advance iov iterator when needed in macvtap_put_user()

[David Miller]

From: Jason Wang <jasowang@redhat.com>
Date: Thu, 20 Nov 2014 16:31:05 +0800

> When mergeable buffer is used, vnet_hdr_sz is greater than sizeof struct
> virtio_net_hdr. So we need advance the iov iterators in this case.
> 
> Fixes 6c36d2e26cda1ad3e2c4b90dd843825fc62fe5b4 ("macvtap: Use iovec iterators")
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Signed-off-by: Jason Wang <jasowang@redhat.com>

Applied, thanks.