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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 20 Nov 2021 16:31:49 -0800 From: Kees Cook <keescook@...omium.org> To: Jakub Kicinski <kuba@...nel.org> Cc: Kees Cook <keescook@...omium.org>, "Gustavo A . R . Silva" <gustavoars@...nel.org>, "Jason A . Donenfeld" <Jason@...c4.com>, "David S. Miller" <davem@...emloft.net>, Jonathan Lemon <jonathan.lemon@...il.com>, Alexander Lobakin <alobakin@...me>, Jakub Sitnicki <jakub@...udflare.com>, Marco Elver <elver@...gle.com>, Willem de Bruijn <willemb@...gle.com>, Alexei Starovoitov <ast@...nel.org>, Daniel Borkmann <daniel@...earbox.net>, Andrii Nakryiko <andrii@...nel.org>, Martin KaFai Lau <kafai@...com>, Song Liu <songliubraving@...com>, Yonghong Song <yhs@...com>, John Fastabend <john.fastabend@...il.com>, KP Singh <kpsingh@...nel.org>, Nathan Chancellor <nathan@...nel.org>, Nick Desaulniers <ndesaulniers@...gle.com>, Eric Dumazet <edumazet@...gle.com>, Cong Wang <cong.wang@...edance.com>, Paolo Abeni <pabeni@...hat.com>, Talal Ahmad <talalahmad@...gle.com>, Kevin Hao <haokexin@...il.com>, Ilias Apalodimas <ilias.apalodimas@...aro.org>, Vasily Averin <vvs@...tuozzo.com>, linux-kernel@...r.kernel.org, wireguard@...ts.zx2c4.com, netdev@...r.kernel.org, bpf@...r.kernel.org, llvm@...ts.linux.dev, linux-hardening@...r.kernel.org Subject: [PATCH v2 net-next 2/2] skbuff: Switch structure bounds to struct_group() In preparation for FORTIFY_SOURCE performing compile-time and run-time field bounds checking for memcpy(), memmove(), and memset(), avoid intentionally writing across neighboring fields. Replace the existing empty member position markers "headers_start" and "headers_end" with a struct_group(). This will allow memcpy() and sizeof() to more easily reason about sizes, and improve readability. "pahole" shows no size nor member offset changes to struct sk_buff. "objdump -d" shows no object code changes (outside of WARNs affected by source line number changes). Signed-off-by: Kees Cook <keescook@...omium.org> Reviewed-by: Gustavo A. R. Silva <gustavoars@...nel.org> Reviewed-by: Jason A. Donenfeld <Jason@...c4.com> # drivers/net/wireguard/* Link: https://lore.kernel.org/lkml/20210728035006.GD35706@embeddedor --- drivers/net/wireguard/queueing.h | 4 +--- include/linux/skbuff.h | 10 +++------- net/core/skbuff.c | 14 +++++--------- 3 files changed, 9 insertions(+), 19 deletions(-) diff --git a/drivers/net/wireguard/queueing.h b/drivers/net/wireguard/queueing.h index 4ef2944a68bc..52da5e963003 100644 --- a/drivers/net/wireguard/queueing.h +++ b/drivers/net/wireguard/queueing.h @@ -79,9 +79,7 @@ static inline void wg_reset_packet(struct sk_buff *skb, bool encapsulating) u8 sw_hash = skb->sw_hash; u32 hash = skb->hash; skb_scrub_packet(skb, true); - memset(&skb->headers_start, 0, - offsetof(struct sk_buff, headers_end) - - offsetof(struct sk_buff, headers_start)); + memset(&skb->headers, 0, sizeof(skb->headers)); if (encapsulating) { skb->l4_hash = l4_hash; skb->sw_hash = sw_hash; diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h index 0bce88ac799a..b474e5bd71cf 100644 --- a/include/linux/skbuff.h +++ b/include/linux/skbuff.h @@ -808,12 +808,10 @@ struct sk_buff { __u8 active_extensions; #endif - /* fields enclosed in headers_start/headers_end are copied + /* Fields enclosed in headers group are copied * using a single memcpy() in __copy_skb_header() */ - /* private: */ - __u32 headers_start[0]; - /* public: */ + struct_group(headers, /* private: */ __u8 __pkt_type_offset[0]; @@ -918,9 +916,7 @@ struct sk_buff { u64 kcov_handle; #endif - /* private: */ - __u32 headers_end[0]; - /* public: */ + ); /* end headers group */ /* These elements must be at the end, see alloc_skb() for details. */ sk_buff_data_t tail; diff --git a/net/core/skbuff.c b/net/core/skbuff.c index ba2f38246f07..3a42b2a3a571 100644 --- a/net/core/skbuff.c +++ b/net/core/skbuff.c @@ -992,12 +992,10 @@ void napi_consume_skb(struct sk_buff *skb, int budget) } EXPORT_SYMBOL(napi_consume_skb); -/* Make sure a field is enclosed inside headers_start/headers_end section */ +/* Make sure a field is contained by headers group */ #define CHECK_SKB_FIELD(field) \ - BUILD_BUG_ON(offsetof(struct sk_buff, field) < \ - offsetof(struct sk_buff, headers_start)); \ - BUILD_BUG_ON(offsetof(struct sk_buff, field) > \ - offsetof(struct sk_buff, headers_end)); \ + BUILD_BUG_ON(offsetof(struct sk_buff, field) != \ + offsetof(struct sk_buff, headers.field)); \ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old) { @@ -1009,14 +1007,12 @@ static void __copy_skb_header(struct sk_buff *new, const struct sk_buff *old) __skb_ext_copy(new, old); __nf_copy(new, old, false); - /* Note : this field could be in headers_start/headers_end section + /* Note : this field could be in the headers group. * It is not yet because we do not want to have a 16 bit hole */ new->queue_mapping = old->queue_mapping; - memcpy(&new->headers_start, &old->headers_start, - offsetof(struct sk_buff, headers_end) - - offsetof(struct sk_buff, headers_start)); + memcpy(&new->headers, &old->headers, sizeof(new->headers)); CHECK_SKB_FIELD(protocol); CHECK_SKB_FIELD(csum); CHECK_SKB_FIELD(hash); -- 2.30.2
Powered by blists - more mailing lists