netdev - Re: [PATCH net v2 9/9] bridge: Prevent possible race condition in br_fdb_change_mac

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: <52B0A86C.1080800@redhat.com>
Date:	Tue, 17 Dec 2013 14:39:24 -0500
From:	Vlad Yasevich <vyasevic@...hat.com>
To:	Toshiaki Makita <makita.toshiaki@....ntt.co.jp>,
	"David S . Miller" <davem@...emloft.net>,
	Stephen Hemminger <stephen@...workplumber.org>,
	netdev@...r.kernel.org
Subject: Re: [PATCH net v2 9/9] bridge: Prevent possible race condition in
 br_fdb_change_mac_address

On 12/17/2013 07:03 AM, Toshiaki Makita wrote:
> br_fdb_change_mac_address() calls fdb_insert()/fdb_delete() without
> br->hash_lock.
> 
> These hash list updates are racy with br_fdb_update()/br_fdb_cleanup().
> 
> Signed-off-by: Toshiaki Makita <makita.toshiaki@....ntt.co.jp>

Good bug fix.

Acked-by: Vlad Yasevich <vyasevic@...hat.com>

-vlad

> ---
>  net/bridge/br_fdb.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/net/bridge/br_fdb.c b/net/bridge/br_fdb.c
> index 38cd29d..05fb8da 100644
> --- a/net/bridge/br_fdb.c
> +++ b/net/bridge/br_fdb.c
> @@ -191,6 +191,8 @@ void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr)
>  	struct net_port_vlans *pv;
>  	u16 vid = 0;
>  
> +	spin_lock_bh(&br->hash_lock);
> +
>  	/* If old entry was unassociated with any port, then delete it. */
>  	f = __br_fdb_get(br, br->dev->dev_addr, 0);
>  	if (f && f->is_local && !f->dst)
> @@ -204,7 +206,7 @@ void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr)
>  	 */
>  	pv = br_get_vlan_info(br);
>  	if (!pv)
> -		return;
> +		goto out;
>  
>  	for_each_set_bit_from(vid, pv->vlan_bitmap, VLAN_N_VID) {
>  		f = __br_fdb_get(br, br->dev->dev_addr, vid);
> @@ -212,6 +214,8 @@ void br_fdb_change_mac_address(struct net_bridge *br, const u8 *newaddr)
>  			fdb_delete_local(br, NULL, f);
>  		fdb_insert(br, NULL, newaddr, vid);
>  	}
> +out:
> +	spin_unlock_bh(&br->hash_lock);
>  }
>  
>  void br_fdb_cleanup(unsigned long _data)
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html