Re: [PATCH 2/2] skbuff: Extract list pointers to silence compiler warnings

[Greg KH <gregkh@linuxfoundation.org>]

On Tue, Mar 29, 2022 at 03:02:56PM -0700, Tadeusz Struk wrote:
> Please apply this to stable 5.10.y, and 5.15.y
> ---8<---
> 
> From: Kees Cook <keescook@chromium.org>
> 
> Upstream commit: 1a2fb220edca ("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 also upsets UBSAN, which throws a runtime object-size-mismatch
> error complaining about skbuff queue helpers, as below, when the kernel
> is built with clang and -fsanitize=undefined flag set:
> 
> UBSAN: object-size-mismatch in ./include/linux/skbuff.h:2023:28
> member access within address ffffc90000cb71c0 with insufficient space
> for an object of type 'struct sk_buff'
> 
> 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>
> Signed-off-by: Tadeusz Struk <tadeusz.struk@linaro.org>
> ---
>  include/linux/skbuff.h | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)

Again, this diff is modified from the original.

I'm not going to take either of these now, sorry,

greg k-h