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:   Thu, 18 Jun 2020 22:40:03 +0300
From:   Vladimir Oltean <olteanv@...il.com>
To:     Cong Wang <xiyou.wangcong@...il.com>
Cc:     netdev <netdev@...r.kernel.org>,
        syzbot+f3a0e80c34b3fc28ac5e@...kaller.appspotmail.com,
        Taehee Yoo <ap420073@...il.com>,
        Dmitry Vyukov <dvyukov@...gle.com>
Subject: Re: [Patch net] net: change addr_list_lock back to static key

Hi Cong, Taehee,

On Tue, 9 Jun 2020 at 00:54, Cong Wang <xiyou.wangcong@...il.com> wrote:
>
> The dynamic key update for addr_list_lock still causes troubles,
> for example the following race condition still exists:
>
> CPU 0:                          CPU 1:
> (RCU read lock)                 (RTNL lock)
> dev_mc_seq_show()               netdev_update_lockdep_key()
>                                   -> lockdep_unregister_key()
>  -> netif_addr_lock_bh()
>
> because lockdep doesn't provide an API to update it atomically.
> Therefore, we have to move it back to static keys and use subclass
> for nest locking like before.
>
> In commit 1a33e10e4a95 ("net: partially revert dynamic lockdep key
> changes"), I already reverted most parts of commit ab92d68fc22f
> ("net: core: add generic lockdep keys").
>
> This patch reverts the rest and also part of commit f3b0a18bb6cb
> ("net: remove unnecessary variables and callback"). After this
> patch, addr_list_lock changes back to using static keys and
> subclasses to satisfy lockdep. Thanks to dev->lower_level, we do
> not have to change back to ->ndo_get_lock_subclass().
>
> And hopefully this reduces some syzbot lockdep noises too.
>
> Reported-by: syzbot+f3a0e80c34b3fc28ac5e@...kaller.appspotmail.com
> Cc: Taehee Yoo <ap420073@...il.com>
> Cc: Dmitry Vyukov <dvyukov@...gle.com>
> Signed-off-by: Cong Wang <xiyou.wangcong@...il.com>
> ---
>  drivers/net/bonding/bond_main.c               |  2 --
>  drivers/net/bonding/bond_options.c            |  2 --
>  drivers/net/hamradio/bpqether.c               |  2 ++
>  drivers/net/macsec.c                          |  5 ++++
>  drivers/net/macvlan.c                         | 13 ++++++--
>  drivers/net/vxlan.c                           |  4 +--
>  .../net/wireless/intersil/hostap/hostap_hw.c  |  3 ++
>  include/linux/netdevice.h                     | 12 +++++---
>  net/8021q/vlan_dev.c                          |  8 +++--
>  net/batman-adv/soft-interface.c               |  2 ++
>  net/bridge/br_device.c                        |  8 +++++
>  net/core/dev.c                                | 30 ++++++++++---------
>  net/core/dev_addr_lists.c                     | 12 ++++----
>  net/core/rtnetlink.c                          |  1 -
>  net/dsa/master.c                              |  4 +++
>  net/netrom/af_netrom.c                        |  2 ++
>  net/rose/af_rose.c                            |  2 ++
>  17 files changed, 76 insertions(+), 36 deletions(-)
>

It's me with the stacked DSA devices again:

[   11.424642] ============================================
[   11.429967] WARNING: possible recursive locking detected
[   11.435295] 5.8.0-rc1-00133-g923e4b5032dd-dirty #208 Not tainted
[   11.441319] --------------------------------------------
[   11.446646] dhcpcd/323 is trying to acquire lock:
[   11.451362] ffff000066dd4268
(&dsa_master_addr_list_lock_key/1){+...}-{2:2}, at:
dev_mc_sync+0x44/0x90
[   11.460713]
[   11.460713] but task is already holding lock:
[   11.466561] ffff00006608c268
(&dsa_master_addr_list_lock_key/1){+...}-{2:2}, at:
dev_mc_sync+0x44/0x90
[   11.475905]
[   11.475905] other info that might help us debug this:
[   11.482450]  Possible unsafe locking scenario:
[   11.482450]
[   11.488386]        CPU0
[   11.490833]        ----
[   11.493280]   lock(&dsa_master_addr_list_lock_key/1);
[   11.498347]   lock(&dsa_master_addr_list_lock_key/1);
[   11.503413]
[   11.503413]  *** DEADLOCK ***
[   11.503413]
[   11.509349]  May be due to missing lock nesting notation
[   11.509349]
[   11.516158] 3 locks held by dhcpcd/323:
[   11.520001]  #0: ffffdbd1381dda18 (rtnl_mutex){+.+.}-{3:3}, at:
rtnl_lock+0x24/0x30
[   11.527688]  #1: ffff00006614b268 (_xmit_ETHER){+...}-{2:2}, at:
dev_set_rx_mode+0x28/0x48
[   11.535987]  #2: ffff00006608c268
(&dsa_master_addr_list_lock_key/1){+...}-{2:2}, at:
dev_mc_sync+0x44/0x90
[   11.545766]
[   11.545766] stack backtrace:
[   11.564098] Call trace:
[   11.566549]  dump_backtrace+0x0/0x1e0
[   11.570220]  show_stack+0x20/0x30
[   11.573544]  dump_stack+0xec/0x158
[   11.576955]  __lock_acquire+0xca0/0x2398
[   11.580886]  lock_acquire+0xe8/0x440
[   11.584469]  _raw_spin_lock_nested+0x64/0x90
[   11.588749]  dev_mc_sync+0x44/0x90
[   11.592159]  dsa_slave_set_rx_mode+0x34/0x50
[   11.596438]  __dev_set_rx_mode+0x60/0xa0
[   11.600369]  dev_mc_sync+0x84/0x90
[   11.603778]  dsa_slave_set_rx_mode+0x34/0x50
[   11.608057]  __dev_set_rx_mode+0x60/0xa0
[   11.611989]  dev_set_rx_mode+0x30/0x48
[   11.615745]  __dev_open+0x10c/0x180
[   11.619240]  __dev_change_flags+0x170/0x1c8
[   11.623432]  dev_change_flags+0x2c/0x70
[   11.627279]  devinet_ioctl+0x774/0x878
[   11.631036]  inet_ioctl+0x348/0x3b0
[   11.634532]  sock_do_ioctl+0x50/0x310
[   11.638202]  sock_ioctl+0x1f8/0x580
[   11.641698]  ksys_ioctl+0xb0/0xf0
[   11.645019]  __arm64_sys_ioctl+0x28/0x38
[   11.648951]  el0_svc_common.constprop.0+0x7c/0x180
[   11.653753]  do_el0_svc+0x2c/0x98
[   11.657075]  el0_sync_handler+0x9c/0x1b8
[   11.661005]  el0_sync+0x158/0x180

Could you please share some suggestions?

Thanks,
-Vladimir

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ