ACK/cmnt: [SRU][Xenial][Zesty][PATCH 1/1] net: reduce skb_warn_bad_offload() noise

[Stefan Bader]

On 30.07.2017 16:42, Joseph Salisbury wrote:
> From: Eric Dumazet <edumazet at google.com>
> 
> BugLink: http://bugs.launchpad.net/bugs/1705447
> 
> Dmitry reported warnings occurring in __skb_gso_segment() [1]
> 
> All SKB_GSO_DODGY producers can allow user space to feed
> packets that trigger the current check.
> 
> We could prevent them from doing so, rejecting packets, but
> this might add regressions to existing programs.
> 
> It turns out our SKB_GSO_DODGY handlers properly set up checksum
> information that is needed anyway when packets needs to be segmented.
> 
> By checking again skb_needs_check() after skb_mac_gso_segment(),
> we should remove these pesky warnings, at a very minor cost.
> 
> With help from Willem de Bruijn
> 
> [1]
> WARNING: CPU: 1 PID: 6768 at net/core/dev.c:2439 skb_warn_bad_offload+0x2af/0x390 net/core/dev.c:2434
> lo: caps=(0x000000a2803b7c69, 0x0000000000000000) len=138 data_len=0 gso_size=15883 gso_type=4 ip_summed=0
> Kernel panic - not syncing: panic_on_warn set ...
> 
> CPU: 1 PID: 6768 Comm: syz-executor1 Not tainted 4.9.0 #5
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011
>  ffff8801c063ecd8 ffffffff82346bdf ffffffff00000001 1ffff100380c7d2e
>  ffffed00380c7d26 0000000041b58ab3 ffffffff84b37e38 ffffffff823468f1
>  ffffffff84820740 ffffffff84f289c0 dffffc0000000000 ffff8801c063ee20
> Call Trace:
>  [<ffffffff82346bdf>] __dump_stack lib/dump_stack.c:15 [inline]
>  [<ffffffff82346bdf>] dump_stack+0x2ee/0x3ef lib/dump_stack.c:51
>  [<ffffffff81827e34>] panic+0x1fb/0x412 kernel/panic.c:179
>  [<ffffffff8141f704>] __warn+0x1c4/0x1e0 kernel/panic.c:542
>  [<ffffffff8141f7e5>] warn_slowpath_fmt+0xc5/0x100 kernel/panic.c:565
>  [<ffffffff8356cbaf>] skb_warn_bad_offload+0x2af/0x390 net/core/dev.c:2434
>  [<ffffffff83585cd2>] __skb_gso_segment+0x482/0x780 net/core/dev.c:2706
>  [<ffffffff83586f19>] skb_gso_segment include/linux/netdevice.h:3985 [inline]
>  [<ffffffff83586f19>] validate_xmit_skb+0x5c9/0xc20 net/core/dev.c:2969
>  [<ffffffff835892bb>] __dev_queue_xmit+0xe6b/0x1e70 net/core/dev.c:3383
>  [<ffffffff8358a2d7>] dev_queue_xmit+0x17/0x20 net/core/dev.c:3424
>  [<ffffffff83ad161d>] packet_snd net/packet/af_packet.c:2930 [inline]
>  [<ffffffff83ad161d>] packet_sendmsg+0x32ed/0x4d30 net/packet/af_packet.c:2955
>  [<ffffffff834f0aaa>] sock_sendmsg_nosec net/socket.c:621 [inline]
>  [<ffffffff834f0aaa>] sock_sendmsg+0xca/0x110 net/socket.c:631
>  [<ffffffff834f329a>] ___sys_sendmsg+0x8fa/0x9f0 net/socket.c:1954
>  [<ffffffff834f5e58>] __sys_sendmsg+0x138/0x300 net/socket.c:1988
>  [<ffffffff834f604d>] SYSC_sendmsg net/socket.c:1999 [inline]
>  [<ffffffff834f604d>] SyS_sendmsg+0x2d/0x50 net/socket.c:1995
>  [<ffffffff84371941>] entry_SYSCALL_64_fastpath+0x1f/0xc2
> 
> Signed-off-by: Eric Dumazet <edumazet at google.com>
> Reported-by: Dmitry Vyukov  <dvyukov at google.com>
> Cc: Willem de Bruijn <willemb at google.com>
> Signed-off-by: David S. Miller <davem at davemloft.net>
> (cherry picked from commit b2504a5dbef3305ef41988ad270b0e8ec289331c)
> Signed-off-by: Joseph Salisbury <joseph.salisbury at canonical.com>
Acked-by: Stefan Bader <stefan.bader at canonical.com>

> ---

The description of the bug report has something that almost looks usable as SRU
justification. Just might need the polish of proper template words.

-Stefan

>  net/core/dev.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 54f8c16..f9f7792 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2702,11 +2702,12 @@ static inline bool skb_needs_check(struct sk_buff *skb, bool tx_path)
>  struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
>  				  netdev_features_t features, bool tx_path)
>  {
> +	struct sk_buff *segs;
> +
>  	if (unlikely(skb_needs_check(skb, tx_path))) {
>  		int err;
>  
> -		skb_warn_bad_offload(skb);
> -
> +		/* We're going to init ->check field in TCP or UDP header */
>  		err = skb_cow_head(skb, 0);
>  		if (err < 0)
>  			return ERR_PTR(err);
> @@ -2734,7 +2735,12 @@ struct sk_buff *__skb_gso_segment(struct sk_buff *skb,
>  	skb_reset_mac_header(skb);
>  	skb_reset_mac_len(skb);
>  
> -	return skb_mac_gso_segment(skb, features);
> +	segs = skb_mac_gso_segment(skb, features);
> +
> +	if (unlikely(skb_needs_check(skb, tx_path)))
> +		skb_warn_bad_offload(skb);
> +
> +	return segs;
>  }
>  EXPORT_SYMBOL(__skb_gso_segment);
>  
> 


-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: OpenPGP digital signature
URL: <https://lists.ubuntu.com/archives/kernel-team/attachments/20170731/c44c7bf3/attachment.sig>