* [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
* 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
* [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 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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).