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  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Fri, 4 Aug 2023 10:36:29 -0400
From: Tyler Stachecki <stachecki.tyler@...il.com>
To: edward.cree@....com
Cc: davem@...emloft.net, kuba@...nel.org, edumazet@...gle.com,
	pabeni@...hat.com, Edward Cree <ecree.xilinx@...il.com>,
	netdev@...r.kernel.org, Martin Habets <habetsm.xilinx@...il.com>
Subject: Re: [RFC PATCH net] net-gro: restore check for NULL skb in
 napi_gro_frags

On Wed, Aug 02, 2023 at 10:23:40AM +0100, edward.cree@....com wrote:
> From: Edward Cree <ecree.xilinx@...il.com>
> 
> Cited commit removed the check on the grounds that napi_gro_frags must
>  not be called by drivers if napi_get_frags failed.  But skb can also
>  be NULL if napi_frags_skb fails to pull the ethernet header ("dropping
>  impossible skb" message).  In this case return GRO_CONSUMED, as
>  otherwise continuing on would cause a NULL dereference panic in
>  dev_gro_receive().
> 
> Fixes: 1d11fa696733 ("net-gro: remove GRO_DROP")
> Reviewed-by: Martin Habets <habetsm.xilinx@...il.com>
> Signed-off-by: Edward Cree <ecree.xilinx@...il.com>
> ---
> An sfc customer has encountered this panic in the wild; we're still
>  investigating exactly how it happened (we have a reproducer) but it
>  seems wise to have the core handle this check rather than requiring
>  it in every driver.
> ---
>  net/core/gro.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/net/core/gro.c b/net/core/gro.c
> index 0759277dc14e..0159972038da 100644
> --- a/net/core/gro.c
> +++ b/net/core/gro.c
> @@ -731,6 +731,9 @@ gro_result_t napi_gro_frags(struct napi_struct *napi)
>  	gro_result_t ret;
>  	struct sk_buff *skb = napi_frags_skb(napi);
>  
> +	if (!skb)
> +		return GRO_CONSUMED;
> +
>  	trace_napi_gro_frags_entry(skb);
>  
>  	ret = napi_frags_finish(napi, skb, dev_gro_receive(napi, skb));

Given that this case is rarely hit, and given that there are some performance
concerns raised wrt this change, it may be beneficial to hint that this
branch is unlikely.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ