Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

skbuff: Extract list pointers to silence compiler warnings

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>
Link: https://lore.kernel.org/r/20211207062758.2324338-1-keescook@chromium.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

authored by

Kees Cook and committed by
Jakub Kicinski
1a2fb220 f20f94f7

+10 -8
+10 -8
include/linux/skbuff.h
··· 292 292 #endif 293 293 294 294 struct sk_buff_head { 295 - /* These two members must be first. */ 296 - struct sk_buff *next; 297 - struct sk_buff *prev; 295 + /* These two members must be first to match sk_buff. */ 296 + struct_group_tagged(sk_buff_list, list, 297 + struct sk_buff *next; 298 + struct sk_buff *prev; 299 + ); 298 300 299 301 __u32 qlen; 300 302 spinlock_t lock; ··· 732 730 struct sk_buff { 733 731 union { 734 732 struct { 735 - /* These two members must be first. */ 733 + /* These two members must be first to match sk_buff_head. */ 736 734 struct sk_buff *next; 737 735 struct sk_buff *prev; 738 736 ··· 1978 1976 */ 1979 1977 WRITE_ONCE(newsk->next, next); 1980 1978 WRITE_ONCE(newsk->prev, prev); 1981 - WRITE_ONCE(next->prev, newsk); 1982 - WRITE_ONCE(prev->next, newsk); 1979 + WRITE_ONCE(((struct sk_buff_list *)next)->prev, newsk); 1980 + WRITE_ONCE(((struct sk_buff_list *)prev)->next, newsk); 1983 1981 WRITE_ONCE(list->qlen, list->qlen + 1); 1984 1982 } 1985 1983 ··· 2075 2073 struct sk_buff *prev, 2076 2074 struct sk_buff *newsk) 2077 2075 { 2078 - __skb_insert(newsk, prev, prev->next, list); 2076 + __skb_insert(newsk, prev, ((struct sk_buff_list *)prev)->next, list); 2079 2077 } 2080 2078 2081 2079 void skb_append(struct sk_buff *old, struct sk_buff *newsk, ··· 2085 2083 struct sk_buff *next, 2086 2084 struct sk_buff *newsk) 2087 2085 { 2088 - __skb_insert(newsk, next->prev, next, list); 2086 + __skb_insert(newsk, ((struct sk_buff_list *)next)->prev, next, list); 2089 2087 } 2090 2088 2091 2089 /**