linux-kernel - Re: [PATCH 3/4] net: initialize online_mask unconditionally in __netif_set_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] [day] [month] [year] [list]

Message-ID: <YznDSKbiDI99Om23@yury-laptop>
Date:   Sun, 2 Oct 2022 09:58:48 -0700
From:   Yury Norov <yury.norov@...il.com>
To:     netdev@...r.kernel.org, "David S . Miller" <davem@...emloft.net>,
        Eric Dumazet <edumazet@...gle.com>,
        Jakub Kicinski <kuba@...nel.org>,
        Paolo Abeni <pabeni@...hat.com>,
        Sebastian Andrzej Siewior <bigeasy@...utronix.de>,
        Menglong Dong <imagedong@...cent.com>,
        Kuniyuki Iwashima <kuniyu@...zon.com>,
        Petr Machata <petrm@...dia.com>, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 3/4] net: initialize online_mask unconditionally in
 __netif_set_xps_queue()

On Sun, Oct 02, 2022 at 08:17:01AM -0700, Yury Norov wrote:
> If the mask is initialized unconditionally, it's possible to use bitmap
> API to traverse it, which is done in the following patch.
> 
> Signed-off-by: Yury Norov <yury.norov@...il.com>
> ---
>  net/core/dev.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/net/core/dev.c b/net/core/dev.c
> index 39a4cc7b3a06..266378ad1cf1 100644
> --- a/net/core/dev.c
> +++ b/net/core/dev.c
> @@ -2542,7 +2542,7 @@ int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
>  			  u16 index, enum xps_map_type type)
>  {
>  	struct xps_dev_maps *dev_maps, *new_dev_maps = NULL, *old_dev_maps = NULL;
> -	const unsigned long *online_mask = NULL;
> +	const unsigned long *online_mask;
>  	bool active = false, copy = false;
>  	int i, j, tci, numa_node_id = -2;
>  	int maps_sz, num_tc = 1, tc = 0;
> @@ -2565,9 +2565,11 @@ int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
>  
>  	if (type == XPS_RXQS) {
>  		nr_ids = dev->num_rx_queues;
> +		online_mask = bitmap_alloc(nr_ids, GFP_KERNEL);
> +		if (!online_mask)
> +			return -ENOMEM;

Oh god, I missed a line here while preparing the patch. It should be:

 +		online_mask = bitmap_alloc(nr_ids, GFP_KERNEL);
 +		if (!online_mask)
 +			return -ENOMEM;
 +              bitmap_fill(online_mask, nr_ids);

I'll send v2 after collecting the comments.

>  	} else {
> -		if (num_possible_cpus() > 1)
> -			online_mask = cpumask_bits(cpu_online_mask);
> +		online_mask = cpumask_bits(cpu_online_mask);
>  		nr_ids = nr_cpu_ids;
>  	}
>  
> @@ -2593,10 +2595,8 @@ int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
>  	     j < nr_ids;) {
>  		if (!new_dev_maps) {
>  			new_dev_maps = kzalloc(maps_sz, GFP_KERNEL);
> -			if (!new_dev_maps) {
> -				mutex_unlock(&xps_map_mutex);
> -				return -ENOMEM;
> -			}
> +			if (!new_dev_maps)
> +				goto err_out;
>  
>  			new_dev_maps->nr_ids = nr_ids;
>  			new_dev_maps->num_tc = num_tc;
> @@ -2718,7 +2718,8 @@ int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
>  
>  out_no_maps:
>  	mutex_unlock(&xps_map_mutex);
> -
> +	if (type == XPS_RXQS)
> +		bitmap_free(online_mask);
>  	return 0;
>  error:
>  	/* remove any maps that we added */
> @@ -2733,8 +2734,10 @@ int __netif_set_xps_queue(struct net_device *dev, const unsigned long *mask,
>  		}
>  	}
>  
> +err_out:
>  	mutex_unlock(&xps_map_mutex);
> -
> +	if (type == XPS_RXQS)
> +		bitmap_free(online_mask);
>  	kfree(new_dev_maps);
>  	return -ENOMEM;
>  }
> -- 
> 2.34.1