netdev - Re: bug report: WARNING in bonding

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]

Date:   Tue, 6 Apr 2021 14:02:37 -0400
From:   Steven Rostedt <rostedt@...dmis.org>
To:     Tariq Toukan <ttoukan.linux@...il.com>
Cc:     Ido Schimmel <idosch@...sch.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "Peter Zijlstra (Intel)" <peterz@...radead.org>,
        Ingo Molnar <mingo@...nel.org>,
        David Miller <davem@...emloft.net>,
        Jakub Kicinski <kuba@...nel.org>,
        Jay Vosburgh <j.vosburgh@...il.com>,
        Veaceslav Falico <vfalico@...il.com>,
        Andy Gospodarek <andy@...yhouse.net>,
        Moshe Shemesh <moshe@...dia.com>,
        Itay Aveksis <itayav@...dia.com>,
        Ran Rozenstein <ranro@...dia.com>,
        Tariq Toukan <tariqt@...dia.com>,
        Saeed Mahameed <saeedm@...dia.com>,
        Leon Romanovsky <leonro@...dia.com>
Subject: Re: bug report: WARNING in bonding

On Thu, 25 Feb 2021 17:24:48 +0200
Tariq Toukan <ttoukan.linux@...il.com> wrote:

> > Hi,
> > 
> > Issue still reproduces. Even in GA kernel.
> > It is always preceded by some other lockdep warning.
> > 
> > So to get the reproduction:
> > - First, have any lockdep issue.
> > - Then, open bond interface.
> > 
> > Any idea what could it be?
> > 
> > We'll share any new info as soon as we have it.

Looks like you are triggering:

int bond_update_slave_arr(struct bonding *bond, struct slave *skipslave)
{
	struct bond_up_slave *usable_slaves = NULL, *all_slaves = NULL;
	struct slave *slave;
	struct list_head *iter;
	int agg_id = 0;
	int ret = 0;

#ifdef CONFIG_LOCKDEP
	WARN_ON(lockdep_is_held(&bond->mode_lock));
#endif

And the below commit made lockdep_is_held() always return true if lockdep
has been previously triggered. That is, if you had a lockdep splat earlier,
then lockdep_is_held() will always return true, and this WARN_ON() will
always trigger.

Peter,

Perhaps we should not have this part of your patch:

@@ -5056,13 +5081,13 @@ noinstr int lock_is_held_type(const struct lockdep_map *lock, int read)
        unsigned long flags;
        int ret = 0;
 
-       if (unlikely(current->lockdep_recursion))
+       if (unlikely(!lockdep_enabled()))
                return 1; /* avoid false negative lockdep_assert_held() */
 
        raw_local_irq_save(flags);
        check_flags(flags);
 

Because that changes how lock_is_held_type() behaves, and it will return
true if there's been an earlier lockdep splat, and any code that has
something like the above is going to fail.

Although, checking if a lot is not held seems rather strange. If anything,
the above should be changed to WARN_ON_ONCE() so that it doesn't constantly
trigger when a lockdep trigger happens.

-- Steve


> > 
> > Regards,
> > Tariq  
> 
> 
> Bisect shows this is the offending commit:
> 
> commit 4d004099a668c41522242aa146a38cc4eb59cb1e
> Author: Peter Zijlstra <peterz@...radead.org>
> Date:   Fri Oct 2 11:04:21 2020 +0200
> 
>      lockdep: Fix lockdep recursion
> 
>      Steve reported that lockdep_assert*irq*(), when nested inside lockdep
>      itself, will trigger a false-positive.
> 
>      One example is the stack-trace code, as called from inside lockdep,
>      triggering tracing, which in turn calls RCU, which then uses
>     lockdep_assert_irqs_disabled().
> 
>      Fixes: a21ee6055c30 ("lockdep: Change hardirq{s_enabled,_context} 
> to per-cpu variables")
>      Reported-by: Steven Rostedt <rostedt@...dmis.org>
>      Signed-off-by: Peter Zijlstra (Intel) <peterz@...radead.org>
>      Signed-off-by: Ingo Molnar <mingo@...nel.org>