netdev - Re: [PATCH net-next] net: remove redundant NULL check in remove_xps

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]

Message-ID: <ZL90FnzgLUAPc1Sk@corigine.com>
Date: Tue, 25 Jul 2023 09:04:54 +0200
From: Simon Horman <simon.horman@...igine.com>
To: Zhengchao Shao <shaozhengchao@...wei.com>
Cc: netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
	kuba@...nel.org, pabeni@...hat.com, kuniyu@...zon.com,
	liuhangbin@...il.com, jiri@...nulli.us, hkallweit1@...il.com,
	andy.ren@...cruise.com, weiyongjun1@...wei.com,
	yuehaibing@...wei.com
Subject: Re: [PATCH net-next] net: remove redundant NULL check in
 remove_xps_queue()

On Mon, Jul 24, 2023 at 10:37:35AM +0800, Zhengchao Shao wrote:
> There are currently two paths that call remove_xps_queue():
> 1. __netif_set_xps_queue -> remove_xps_queue
> 2. clean_xps_maps -> remove_xps_queue_cpu -> remove_xps_queue
> There is no need to check dev_maps in remove_xps_queue() because
> dev_maps has been checked on these two paths.
> 
> Signed-off-by: Zhengchao Shao <shaozhengchao@...wei.com>

I have verified the reasoning above is correct.
I am, however, slightly less sure that this is a good idea.

> ---
>  net/core/dev.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index f95e0674570f..76a91b849829 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2384,8 +2384,7 @@ static bool remove_xps_queue(struct xps_dev_maps *dev_maps,
>  	struct xps_map *map = NULL;
>  	int pos;
>  
> -	if (dev_maps)
> -		map = xmap_dereference(dev_maps->attr_map[tci]);
> +	map = xmap_dereference(dev_maps->attr_map[tci]);
>  	if (!map)
>  		return false;
>  
> -- 
> 2.34.1
>