netdev - Re: [PATCH net] bridge: mcast: Fail MDB get request on empty entry

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <7ff94e87-1b9d-41e6-82a0-c13ff986adf5@blackwall.org>
Date: Mon, 30 Sep 2024 13:06:23 +0300
From: Nikolay Aleksandrov <razor@...ckwall.org>
To: Ido Schimmel <idosch@...dia.com>, netdev@...r.kernel.org
Cc: davem@...emloft.net, kuba@...nel.org, pabeni@...hat.com,
 edumazet@...gle.com, roopa@...dia.com, bridge@...ts.linux.dev,
 jamie.bainbridge@...il.com
Subject: Re: [PATCH net] bridge: mcast: Fail MDB get request on empty entry

On 9/29/24 15:36, Ido Schimmel wrote:
> When user space deletes a port from an MDB entry, the port is removed
> synchronously. If this was the last port in the entry and the entry is
> not joined by the host itself, then the entry is scheduled for deletion
> via a timer.
> 
> The above means that it is possible for the MDB get netlink request to
> retrieve an empty entry which is scheduled for deletion. This is
> problematic as after deleting the last port in an entry, user space
> cannot rely on a non-zero return code from the MDB get request as an
> indication that the port was successfully removed.
> 
> Fix by returning an error when the entry's port list is empty and the
> entry is not joined by the host.
> 
> Fixes: 68b380a395a7 ("bridge: mcast: Add MDB get support")
> Reported-by: Jamie Bainbridge <jamie.bainbridge@...il.com>
> Closes: https://lore.kernel.org/netdev/c92569919307749f879b9482b0f3e125b7d9d2e3.1726480066.git.jamie.bainbridge@gmail.com/
> Tested-by: Jamie Bainbridge <jamie.bainbridge@...il.com>
> Signed-off-by: Ido Schimmel <idosch@...dia.com>
> ---
>  net/bridge/br_mdb.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/net/bridge/br_mdb.c b/net/bridge/br_mdb.c
> index bc37e47ad829..1a52a0bca086 100644
> --- a/net/bridge/br_mdb.c
> +++ b/net/bridge/br_mdb.c
> @@ -1674,7 +1674,7 @@ int br_mdb_get(struct net_device *dev, struct nlattr *tb[], u32 portid, u32 seq,
>  	spin_lock_bh(&br->multicast_lock);
>  
>  	mp = br_mdb_ip_get(br, &group);
> -	if (!mp) {
> +	if (!mp || (!mp->ports && !mp->host_joined)) {
>  		NL_SET_ERR_MSG_MOD(extack, "MDB entry not found");
>  		err = -ENOENT;
>  		goto unlock;

Acked-by: Nikolay Aleksandrov <razor@...ckwall.org>