* [PATCH] skbuff: Extract list pointers to silence compiler warnings
@ 2021-12-07 6:27 Kees Cook
2021-12-09 21:30 ` patchwork-bot+netdevbpf
0 siblings, 1 reply; 2+ messages in thread
From: Kees Cook @ 2021-12-07 6:27 UTC (permalink / raw)
To: Jakub Kicinski
Cc: Kees Cook, David S. Miller, Jonathan Lemon, Eric Dumazet,
Marco Elver, Alexander Lobakin, Paolo Abeni, Cong Wang,
Talal Ahmad, Kevin Hao, netdev, linux-kernel, linux-hardening
Under both -Warray-bounds and the object_size sanitizer, the compiler is
upset about accessing prev/next of sk_buff when the object it thinks it
is coming from is sk_buff_head. The warning is a false positive due to
the compiler taking a conservative approach, opting to warn at casting
time rather than access time.
However, in support of enabling -Warray-bounds globally (which has
found many real bugs), arrange things for sk_buff so that the compiler
can unambiguously see that there is no intention to access anything
except prev/next. Introduce and cast to a separate struct sk_buff_list,
which contains _only_ the first two fields, silencing the warnings:
In file included from ./include/net/net_namespace.h:39,
from ./include/linux/netdevice.h:37,
from net/core/netpoll.c:17:
net/core/netpoll.c: In function 'refill_skbs':
./include/linux/skbuff.h:2086:9: warning: array subscript 'struct sk_buff[0]' is partly outside array bounds of 'struct sk_buff_head[1]' [-Warray-bounds]
2086 | __skb_insert(newsk, next->prev, next, list);
| ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
net/core/netpoll.c:49:28: note: while referencing 'skb_pool'
49 | static struct sk_buff_head skb_pool;
| ^~~~~~~~
This change results in no executable instruction differences.
Signed-off-by: Kees Cook <keescook@chromium.org>
---
include/linux/skbuff.h | 18 ++++++++++--------
1 file changed, 10 insertions(+), 8 deletions(-)
diff --git a/include/linux/skbuff.h b/include/linux/skbuff.h
index eae4bd3237a4..ec71b45b62b0 100644
--- a/include/linux/skbuff.h
+++ b/include/linux/skbuff.h
@@ -292,9 +292,11 @@ struct tc_skb_ext {
#endif
struct sk_buff_head {
- /* These two members must be first. */
- struct sk_buff *next;
- struct sk_buff *prev;
+ /* These two members must be first to match sk_buff. */
+ struct_group_tagged(sk_buff_list, list,
+ struct sk_buff *next;
+ struct sk_buff *prev;
+ );
__u32 qlen;
spinlock_t lock;
@@ -730,7 +732,7 @@ typedef unsigned char *sk_buff_data_t;
struct sk_buff {
union {
struct {
- /* These two members must be first. */
+ /* These two members must be first to match sk_buff_head. */
struct sk_buff *next;
struct sk_buff *prev;
@@ -1976,8 +1978,8 @@ static inline void __skb_insert(struct sk_buff *newsk,
*/
WRITE_ONCE(newsk->next, next);
WRITE_ONCE(newsk->prev, prev);
- WRITE_ONCE(next->prev, newsk);
- WRITE_ONCE(prev->next, newsk);
+ WRITE_ONCE(((struct sk_buff_list *)next)->prev, newsk);
+ WRITE_ONCE(((struct sk_buff_list *)prev)->next, newsk);
WRITE_ONCE(list->qlen, list->qlen + 1);
}
@@ -2073,7 +2075,7 @@ static inline void __skb_queue_after(struct sk_buff_head *list,
struct sk_buff *prev,
struct sk_buff *newsk)
{
- __skb_insert(newsk, prev, prev->next, list);
+ __skb_insert(newsk, prev, ((struct sk_buff_list *)prev)->next, list);
}
void skb_append(struct sk_buff *old, struct sk_buff *newsk,
@@ -2083,7 +2085,7 @@ static inline void __skb_queue_before(struct sk_buff_head *list,
struct sk_buff *next,
struct sk_buff *newsk)
{
- __skb_insert(newsk, next->prev, next, list);
+ __skb_insert(newsk, ((struct sk_buff_list *)next)->prev, next, list);
}
/**
--
2.30.2
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] skbuff: Extract list pointers to silence compiler warnings
2021-12-07 6:27 [PATCH] skbuff: Extract list pointers to silence compiler warnings Kees Cook
@ 2021-12-09 21:30 ` patchwork-bot+netdevbpf
0 siblings, 0 replies; 2+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-12-09 21:30 UTC (permalink / raw)
To: Kees Cook
Cc: kuba, davem, jonathan.lemon, edumazet, elver, alobakin, pabeni,
cong.wang, talalahmad, haokexin, netdev, linux-kernel,
linux-hardening
Hello:
This patch was applied to netdev/net-next.git (master)
by Jakub Kicinski <kuba@kernel.org>:
On Mon, 6 Dec 2021 22:27:58 -0800 you wrote:
> Under both -Warray-bounds and the object_size sanitizer, the compiler is
> upset about accessing prev/next of sk_buff when the object it thinks it
> is coming from is sk_buff_head. The warning is a false positive due to
> the compiler taking a conservative approach, opting to warn at casting
> time rather than access time.
>
> However, in support of enabling -Warray-bounds globally (which has
> found many real bugs), arrange things for sk_buff so that the compiler
> can unambiguously see that there is no intention to access anything
> except prev/next. Introduce and cast to a separate struct sk_buff_list,
> which contains _only_ the first two fields, silencing the warnings:
>
> [...]
Here is the summary with links:
- skbuff: Extract list pointers to silence compiler warnings
https://git.kernel.org/netdev/net-next/c/1a2fb220edca
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2021-12-09 21:30 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-12-07 6:27 [PATCH] skbuff: Extract list pointers to silence compiler warnings Kees Cook
2021-12-09 21:30 ` patchwork-bot+netdevbpf
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.