linux-kernel - Re: [PATCH net-next v2 2/2] net: reorder dev_addr

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: <20250313175603.17045-1-kuniyu@amazon.com>
Date: Thu, 13 Mar 2025 10:56:00 -0700
From: Kuniyuki Iwashima <kuniyu@...zon.com>
To: <sdf@...ichev.me>
CC: <andrew+netdev@...n.ch>, <atenart@...nel.org>, <davem@...emloft.net>,
	<edumazet@...gle.com>, <enjuk@...zon.com>, <horms@...nel.org>,
	<jasowang@...hat.com>, <jdamato@...tly.com>, <kory.maincent@...tlin.com>,
	<kuba@...nel.org>, <kuniyu@...zon.com>, <linux-kernel@...r.kernel.org>,
	<netdev@...r.kernel.org>, <pabeni@...hat.com>,
	<willemdebruijn.kernel@...il.com>
Subject: Re: [PATCH net-next v2 2/2] net: reorder dev_addr_sem lock

From: Stanislav Fomichev <sdf@...ichev.me>
Date: Wed, 12 Mar 2025 12:05:13 -0700
> Lockdep complains about circular lock in 1 -> 2 -> 3 (see below).
> 
> Change the lock ordering to be:
> - rtnl_lock
> - dev_addr_sem
> - netdev_ops (only for lower devices!)
> - team_lock (or other per-upper device lock)
> 
> 1. rtnl_lock -> netdev_ops -> dev_addr_sem
> 
> rtnl_setlink
>   rtnl_lock
>     do_setlink IFLA_ADDRESS on lower
>       netdev_ops
>         dev_addr_sem
> 
> 2. rtnl_lock -> team_lock -> netdev_ops
> 
> rtnl_newlink
>   rtnl_lock
>     do_setlink IFLA_MASTER on lower
>       do_set_master
>         team_add_slave
>           team_lock
>             team_port_add
> 	      dev_set_mtu
> 	        netdev_ops
> 
> 3. rtnl_lock -> dev_addr_sem -> team_lock
> 
> rtnl_newlink
>   rtnl_lock
>     do_setlink IFLA_ADDRESS on upper
>       dev_addr_sem
>         netif_set_mac_address
>           team_set_mac_address
>             team_lock
> 
> 4. rtnl_lock -> netdev_ops -> dev_addr_sem
> 
> rtnl_lock
>   dev_ifsioc
>     dev_set_mac_address_user
> 
> __tun_chr_ioctl
>   rtnl_lock
>     dev_set_mac_address_user
> 
> tap_ioctl
>   rtnl_lock
>     dev_set_mac_address_user
> 
> dev_set_mac_address_user
>   netdev_lock_ops
>     netif_set_mac_address_user
>       dev_addr_sem
> 
> v2:
> - move lock reorder to happen after kmalloc (Kuniyuki)

My intention was move kmalloc() and memcpy() out of both
netdev_lock and dev_addr_sem like

  netdev_ops_unlock()
  kmalloc()
  memcpy()
  down_write()

, but not a big deal :)

> 
> Cc: Kohei Enju <enjuk@...zon.com>
> Fixes: df43d8bf1031 ("net: replace dev_addr_sem with netdev instance lock")
> Signed-off-by: Stanislav Fomichev <sdf@...ichev.me>

Reviewed-by: Kuniyuki Iwashima <kuniyu@...zon.com>