* [PATCH net 0/2] read vnet_hdr_sz once
@ 2017-02-03 23:20 Willem de Bruijn
2017-02-03 23:20 ` [PATCH net 1/2] tun: " Willem de Bruijn
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: Willem de Bruijn @ 2017-02-03 23:20 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
Tuntap devices allow concurrent use and update of field vnet_hdr_sz.
Read the field once to avoid TOCTOU.
Willem de Bruijn (2):
tun: read vnet_hdr_sz once
macvtap: read vnet_hdr_size once
drivers/net/macvtap.c | 4 ++--
drivers/net/tun.c | 10 ++++++----
2 files changed, 8 insertions(+), 6 deletions(-)
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH net 1/2] tun: read vnet_hdr_sz once
2017-02-03 23:20 [PATCH net 0/2] read vnet_hdr_sz once Willem de Bruijn
@ 2017-02-03 23:20 ` Willem de Bruijn
2017-02-03 23:27 ` Eric Dumazet
2017-02-03 23:20 ` [PATCH net 2/2] macvtap: read vnet_hdr_size once Willem de Bruijn
2017-02-07 3:48 ` [PATCH net 0/2] read vnet_hdr_sz once David Miller
2 siblings, 1 reply; 6+ messages in thread
From: Willem de Bruijn @ 2017-02-03 23:20 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn, Eric Dumazet
From: Willem de Bruijn <willemb@google.com>
When IFF_VNET_HDR is enabled, a virtio_net header must precede data.
Data length is verified to be greater than or equal to expected header
length tun->vnet_hdr_sz before copying.
Read this value once and cache locally, as it can be updated between
the test and use (TOCTOU).
Signed-off-by: Willem de Bruijn <willemb@google.com>
Reported-by: Dmitry Vyukov <dvyukov@google.com>
CC: Eric Dumazet <edumazet@google.com>
---
drivers/net/tun.c | 10 ++++++----
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/net/tun.c b/drivers/net/tun.c
index 2cd10b26b650..bfabe180053e 100644
--- a/drivers/net/tun.c
+++ b/drivers/net/tun.c
@@ -1170,9 +1170,11 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
}
if (tun->flags & IFF_VNET_HDR) {
- if (len < tun->vnet_hdr_sz)
+ int vnet_hdr_sz = READ_ONCE(tun->vnet_hdr_sz);
+
+ if (len < vnet_hdr_sz)
return -EINVAL;
- len -= tun->vnet_hdr_sz;
+ len -= vnet_hdr_sz;
if (!copy_from_iter_full(&gso, sizeof(gso), from))
return -EFAULT;
@@ -1183,7 +1185,7 @@ static ssize_t tun_get_user(struct tun_struct *tun, struct tun_file *tfile,
if (tun16_to_cpu(tun, gso.hdr_len) > len)
return -EINVAL;
- iov_iter_advance(from, tun->vnet_hdr_sz - sizeof(gso));
+ iov_iter_advance(from, vnet_hdr_sz - sizeof(gso));
}
if ((tun->flags & TUN_TYPE_MASK) == IFF_TAP) {
@@ -1335,7 +1337,7 @@ static ssize_t tun_put_user(struct tun_struct *tun,
vlan_hlen = VLAN_HLEN;
if (tun->flags & IFF_VNET_HDR)
- vnet_hdr_sz = tun->vnet_hdr_sz;
+ vnet_hdr_sz = READ_ONCE(tun->vnet_hdr_sz);
total = skb->len + vlan_hlen + vnet_hdr_sz;
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH net 2/2] macvtap: read vnet_hdr_size once
2017-02-03 23:20 [PATCH net 0/2] read vnet_hdr_sz once Willem de Bruijn
2017-02-03 23:20 ` [PATCH net 1/2] tun: " Willem de Bruijn
@ 2017-02-03 23:20 ` Willem de Bruijn
2017-02-03 23:27 ` Eric Dumazet
2017-02-07 3:48 ` [PATCH net 0/2] read vnet_hdr_sz once David Miller
2 siblings, 1 reply; 6+ messages in thread
From: Willem de Bruijn @ 2017-02-03 23:20 UTC (permalink / raw)
To: netdev; +Cc: davem, Willem de Bruijn
From: Willem de Bruijn <willemb@google.com>
When IFF_VNET_HDR is enabled, a virtio_net header must precede data.
Data length is verified to be greater than or equal to expected header
length tun->vnet_hdr_sz before copying.
Macvtap functions read the value once, but unless READ_ONCE is used,
the compiler may ignore this and read multiple times. Enforce a single
read and locally cached value to avoid updates between test and use.
Signed-off-by: Willem de Bruijn <willemb@google.com>
Suggested-by: Eric Dumazet <edumazet@google.com>
---
drivers/net/macvtap.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/drivers/net/macvtap.c b/drivers/net/macvtap.c
index 402618565838..c27011bbe30c 100644
--- a/drivers/net/macvtap.c
+++ b/drivers/net/macvtap.c
@@ -681,7 +681,7 @@ static ssize_t macvtap_get_user(struct macvtap_queue *q, struct msghdr *m,
size_t linear;
if (q->flags & IFF_VNET_HDR) {
- vnet_hdr_len = q->vnet_hdr_sz;
+ vnet_hdr_len = READ_ONCE(q->vnet_hdr_sz);
err = -EINVAL;
if (len < vnet_hdr_len)
@@ -820,7 +820,7 @@ static ssize_t macvtap_put_user(struct macvtap_queue *q,
if (q->flags & IFF_VNET_HDR) {
struct virtio_net_hdr vnet_hdr;
- vnet_hdr_len = q->vnet_hdr_sz;
+ vnet_hdr_len = READ_ONCE(q->vnet_hdr_sz);
if (iov_iter_count(iter) < vnet_hdr_len)
return -EINVAL;
--
2.11.0.483.g087da7b7c-goog
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH net 1/2] tun: read vnet_hdr_sz once
2017-02-03 23:20 ` [PATCH net 1/2] tun: " Willem de Bruijn
@ 2017-02-03 23:27 ` Eric Dumazet
0 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2017-02-03 23:27 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, davem, Willem de Bruijn, Eric Dumazet
On Fri, 2017-02-03 at 18:20 -0500, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> When IFF_VNET_HDR is enabled, a virtio_net header must precede data.
> Data length is verified to be greater than or equal to expected header
> length tun->vnet_hdr_sz before copying.
>
> Read this value once and cache locally, as it can be updated between
> the test and use (TOCTOU).
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>
> CC: Eric Dumazet <edumazet@google.com>
> ---
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 2/2] macvtap: read vnet_hdr_size once
2017-02-03 23:20 ` [PATCH net 2/2] macvtap: read vnet_hdr_size once Willem de Bruijn
@ 2017-02-03 23:27 ` Eric Dumazet
0 siblings, 0 replies; 6+ messages in thread
From: Eric Dumazet @ 2017-02-03 23:27 UTC (permalink / raw)
To: Willem de Bruijn; +Cc: netdev, davem, Willem de Bruijn
On Fri, 2017-02-03 at 18:20 -0500, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
>
> When IFF_VNET_HDR is enabled, a virtio_net header must precede data.
> Data length is verified to be greater than or equal to expected header
> length tun->vnet_hdr_sz before copying.
>
> Macvtap functions read the value once, but unless READ_ONCE is used,
> the compiler may ignore this and read multiple times. Enforce a single
> read and locally cached value to avoid updates between test and use.
>
> Signed-off-by: Willem de Bruijn <willemb@google.com>
> Suggested-by: Eric Dumazet <edumazet@google.com>
> ---
Acked-by: Eric Dumazet <edumazet@google.com>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH net 0/2] read vnet_hdr_sz once
2017-02-03 23:20 [PATCH net 0/2] read vnet_hdr_sz once Willem de Bruijn
2017-02-03 23:20 ` [PATCH net 1/2] tun: " Willem de Bruijn
2017-02-03 23:20 ` [PATCH net 2/2] macvtap: read vnet_hdr_size once Willem de Bruijn
@ 2017-02-07 3:48 ` David Miller
2 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2017-02-07 3:48 UTC (permalink / raw)
To: willemdebruijn.kernel; +Cc: netdev, willemb
From: Willem de Bruijn <willemdebruijn.kernel@gmail.com>
Date: Fri, 3 Feb 2017 18:20:47 -0500
> Tuntap devices allow concurrent use and update of field vnet_hdr_sz.
> Read the field once to avoid TOCTOU.
Series applied and queued up for -stable, thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-02-07 3:48 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-02-03 23:20 [PATCH net 0/2] read vnet_hdr_sz once Willem de Bruijn
2017-02-03 23:20 ` [PATCH net 1/2] tun: " Willem de Bruijn
2017-02-03 23:27 ` Eric Dumazet
2017-02-03 23:20 ` [PATCH net 2/2] macvtap: read vnet_hdr_size once Willem de Bruijn
2017-02-03 23:27 ` Eric Dumazet
2017-02-07 3:48 ` [PATCH net 0/2] read vnet_hdr_sz once David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.