netdev - Re: [net-next PATCH v6 1/2] octeontx2-af: Add new mbox to support multicast/mirror offload

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: Wed, 06 Dec 2023 11:58:32 +0100
From: Paolo Abeni <pabeni@...hat.com>
To: Suman Ghosh <sumang@...vell.com>, sgoutham@...vell.com,
 gakula@...vell.com,  sbhatta@...vell.com, hkelam@...vell.com,
 davem@...emloft.net, edumazet@...gle.com,  kuba@...nel.org,
 netdev@...r.kernel.org, linux-kernel@...r.kernel.org, 
 lcherian@...vell.com, jerinj@...vell.com, horms@...nel.org, 
 wojciech.drewek@...el.com
Subject: Re: [net-next PATCH v6 1/2] octeontx2-af: Add new mbox to support
 multicast/mirror offload

On Mon, 2023-12-04 at 19:49 +0530, Suman Ghosh wrote:
> A new mailbox is added to support offloading of multicast/mirror
> functionality. The mailbox also supports dynamic updation of the
> multicast/mirror list.
> 
> Signed-off-by: Suman Ghosh <sumang@...vell.com>
> Reviewed-by: Wojciech Drewek <wojciech.drewek@...el.com>
> Reviewed-by: Simon Horman <horms@...nel.org>

Note that v5 was already applied to net-next. But I still have a
relevant note, see below.

> @@ -5797,3 +6127,337 @@ int rvu_mbox_handler_nix_bandprof_get_hwinfo(struct rvu *rvu, struct msg_req *re
>  
>  	return 0;
>  }
> +
> +static struct nix_mcast_grp_elem *rvu_nix_mcast_find_grp_elem(struct nix_mcast_grp *mcast_grp,
> +							      u32 mcast_grp_idx)
> +{
> +	struct nix_mcast_grp_elem *iter;
> +	bool is_found = false;
> +
> +	mutex_lock(&mcast_grp->mcast_grp_lock);
> +	list_for_each_entry(iter, &mcast_grp->mcast_grp_head, list) {
> +		if (iter->mcast_grp_idx == mcast_grp_idx) {
> +			is_found = true;
> +			break;
> +		}
> +	}
> +	mutex_unlock(&mcast_grp->mcast_grp_lock);

AFAICS, at this point another thread/CPU could kick-in and run
rvu_mbox_handler_nix_mcast_grp_destroy() up to completion, freeing
'iter' before it's later used by the current thread.

What prevents such scenario?

_If_ every mcast group manipulation happens under the rtnl lock, then
you could as well completely remove the confusing mcast_grp_lock.

Cheers,

Paolo