All of lore.kernel.org
 help / color / mirror / Atom feed
* Re: [PATCH] lockdep: Add debug printk() for downgrade_write() warning.
       [not found] ` <e1a38e21-d5fe-dee3-7081-bc1a12965a68@i-love.sakura.ne.jp>
@ 2019-01-07  4:19   ` Andrew Morton
  2019-01-07  5:58       ` Dmitry Vyukov
  0 siblings, 1 reply; 6+ messages in thread
From: Andrew Morton @ 2019-01-07  4:19 UTC (permalink / raw)
  To: Tetsuo Handa
  Cc: linux-mm, Dmitry Vyukov, Ingo Molnar, Peter Zijlstra, Will Deacon

On Sun, 6 Jan 2019 19:56:59 +0900 Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote:

> syzbot is frequently hitting downgrade_write(&mm->mmap_sem) warning from
> munmap() request, but I don't know why it is happening. Since lockdep is
> not printing enough information, let's print more. This patch is meant for
> linux-next.git only and will be removed after the problem is solved.
> 
> --- a/kernel/locking/lockdep.c
> +++ b/kernel/locking/lockdep.c
> @@ -50,6 +50,7 @@
>  #include <linux/random.h>
>  #include <linux/jhash.h>
>  #include <linux/nmi.h>
> +#include <linux/rwsem.h>
>  
>  #include <asm/sections.h>
>  
> @@ -3550,6 +3551,24 @@ static int __lock_downgrade(struct lockdep_map *lock, unsigned long ip)
>  	curr->lockdep_depth = i;
>  	curr->curr_chain_key = hlock->prev_chain_key;
>  
> +#if defined(CONFIG_RWSEM_XCHGADD_ALGORITHM) && defined(CONFIG_DEBUG_AID_FOR_SYZBOT)
> +	if (hlock->read && curr->mm) {
> +		struct rw_semaphore *sem = container_of(lock,
> +							struct rw_semaphore,
> +							dep_map);
> +
> +		if (sem == &curr->mm->mmap_sem) {
> +#if defined(CONFIG_RWSEM_SPIN_ON_OWNER)
> +			pr_warn("mmap_sem: hlock->read=%d count=%ld current=%px, owner=%px\n",
> +				hlock->read, atomic_long_read(&sem->count),
> +				curr, READ_ONCE(sem->owner));
> +#else
> +			pr_warn("mmap_sem: hlock->read=%d count=%ld\n",
> +				hlock->read, atomic_long_read(&sem->count));
> +#endif
> +		}
> +	}
> +#endif
>  	WARN(hlock->read, "downgrading a read lock");
>  	hlock->read = 1;
>  	hlock->acquire_ip = ip;

I tossed it in there.

But I wonder if anyone is actually running this code.  Because

--- a/lib/Kconfig.debug~info-task-hung-in-generic_file_write_iter
+++ a/lib/Kconfig.debug
@@ -2069,6 +2069,12 @@ config IO_STRICT_DEVMEM
 
 	  If in doubt, say Y.
 
+config DEBUG_AID_FOR_SYZBOT
+       bool "Additional debug code for syzbot"
+       default n
+       help
+         This option is intended for testing by syzbot.
+

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] lockdep: Add debug printk() for downgrade_write() warning.
@ 2019-01-07  5:58       ` Dmitry Vyukov
  0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Vyukov @ 2019-01-07  5:58 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Tetsuo Handa, Linux-MM, Ingo Molnar, Peter Zijlstra, Will Deacon

On Mon, Jan 7, 2019 at 5:19 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Sun, 6 Jan 2019 19:56:59 +0900 Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> > syzbot is frequently hitting downgrade_write(&mm->mmap_sem) warning from
> > munmap() request, but I don't know why it is happening. Since lockdep is
> > not printing enough information, let's print more. This patch is meant for
> > linux-next.git only and will be removed after the problem is solved.
> >
> > --- a/kernel/locking/lockdep.c
> > +++ b/kernel/locking/lockdep.c
> > @@ -50,6 +50,7 @@
> >  #include <linux/random.h>
> >  #include <linux/jhash.h>
> >  #include <linux/nmi.h>
> > +#include <linux/rwsem.h>
> >
> >  #include <asm/sections.h>
> >
> > @@ -3550,6 +3551,24 @@ static int __lock_downgrade(struct lockdep_map *lock, unsigned long ip)
> >       curr->lockdep_depth = i;
> >       curr->curr_chain_key = hlock->prev_chain_key;
> >
> > +#if defined(CONFIG_RWSEM_XCHGADD_ALGORITHM) && defined(CONFIG_DEBUG_AID_FOR_SYZBOT)
> > +     if (hlock->read && curr->mm) {
> > +             struct rw_semaphore *sem = container_of(lock,
> > +                                                     struct rw_semaphore,
> > +                                                     dep_map);
> > +
> > +             if (sem == &curr->mm->mmap_sem) {
> > +#if defined(CONFIG_RWSEM_SPIN_ON_OWNER)
> > +                     pr_warn("mmap_sem: hlock->read=%d count=%ld current=%px, owner=%px\n",
> > +                             hlock->read, atomic_long_read(&sem->count),
> > +                             curr, READ_ONCE(sem->owner));
> > +#else
> > +                     pr_warn("mmap_sem: hlock->read=%d count=%ld\n",
> > +                             hlock->read, atomic_long_read(&sem->count));
> > +#endif
> > +             }
> > +     }
> > +#endif
> >       WARN(hlock->read, "downgrading a read lock");
> >       hlock->read = 1;
> >       hlock->acquire_ip = ip;
>
> I tossed it in there.
>
> But I wonder if anyone is actually running this code.  Because
>
> --- a/lib/Kconfig.debug~info-task-hung-in-generic_file_write_iter
> +++ a/lib/Kconfig.debug
> @@ -2069,6 +2069,12 @@ config IO_STRICT_DEVMEM
>
>           If in doubt, say Y.
>
> +config DEBUG_AID_FOR_SYZBOT
> +       bool "Additional debug code for syzbot"
> +       default n
> +       help
> +         This option is intended for testing by syzbot.
> +


Yes, syzbot always defines this option:

https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kasan.config#L14
https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kmsan.config#L9

It's meant specifically for such cases.

Tetsuo already got some useful information for past bugs using this feature.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] lockdep: Add debug printk() for downgrade_write() warning.
@ 2019-01-07  5:58       ` Dmitry Vyukov
  0 siblings, 0 replies; 6+ messages in thread
From: Dmitry Vyukov @ 2019-01-07  5:58 UTC (permalink / raw)
  To: Andrew Morton
  Cc: Tetsuo Handa, Linux-MM, Ingo Molnar, Peter Zijlstra, Will Deacon

On Mon, Jan 7, 2019 at 5:19 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Sun, 6 Jan 2019 19:56:59 +0900 Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp> wrote:
>
> > syzbot is frequently hitting downgrade_write(&mm->mmap_sem) warning from
> > munmap() request, but I don't know why it is happening. Since lockdep is
> > not printing enough information, let's print more. This patch is meant for
> > linux-next.git only and will be removed after the problem is solved.
> >
> > --- a/kernel/locking/lockdep.c
> > +++ b/kernel/locking/lockdep.c
> > @@ -50,6 +50,7 @@
> >  #include <linux/random.h>
> >  #include <linux/jhash.h>
> >  #include <linux/nmi.h>
> > +#include <linux/rwsem.h>
> >
> >  #include <asm/sections.h>
> >
> > @@ -3550,6 +3551,24 @@ static int __lock_downgrade(struct lockdep_map *lock, unsigned long ip)
> >       curr->lockdep_depth = i;
> >       curr->curr_chain_key = hlock->prev_chain_key;
> >
> > +#if defined(CONFIG_RWSEM_XCHGADD_ALGORITHM) && defined(CONFIG_DEBUG_AID_FOR_SYZBOT)
> > +     if (hlock->read && curr->mm) {
> > +             struct rw_semaphore *sem = container_of(lock,
> > +                                                     struct rw_semaphore,
> > +                                                     dep_map);
> > +
> > +             if (sem == &curr->mm->mmap_sem) {
> > +#if defined(CONFIG_RWSEM_SPIN_ON_OWNER)
> > +                     pr_warn("mmap_sem: hlock->read=%d count=%ld current=%px, owner=%px\n",
> > +                             hlock->read, atomic_long_read(&sem->count),
> > +                             curr, READ_ONCE(sem->owner));
> > +#else
> > +                     pr_warn("mmap_sem: hlock->read=%d count=%ld\n",
> > +                             hlock->read, atomic_long_read(&sem->count));
> > +#endif
> > +             }
> > +     }
> > +#endif
> >       WARN(hlock->read, "downgrading a read lock");
> >       hlock->read = 1;
> >       hlock->acquire_ip = ip;
>
> I tossed it in there.
>
> But I wonder if anyone is actually running this code.  Because
>
> --- a/lib/Kconfig.debug~info-task-hung-in-generic_file_write_iter
> +++ a/lib/Kconfig.debug
> @@ -2069,6 +2069,12 @@ config IO_STRICT_DEVMEM
>
>           If in doubt, say Y.
>
> +config DEBUG_AID_FOR_SYZBOT
> +       bool "Additional debug code for syzbot"
> +       default n
> +       help
> +         This option is intended for testing by syzbot.
> +


Yes, syzbot always defines this option:

https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kasan.config#L14
https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kmsan.config#L9

It's meant specifically for such cases.

Tetsuo already got some useful information for past bugs using this feature.


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] lockdep: Add debug printk() for downgrade_write() warning.
  2019-01-07  5:58       ` Dmitry Vyukov
  (?)
@ 2019-01-09 11:47       ` Tetsuo Handa
  2019-01-09 18:22         ` Waiman Long
  -1 siblings, 1 reply; 6+ messages in thread
From: Tetsuo Handa @ 2019-01-09 11:47 UTC (permalink / raw)
  To: Peter Zijlstra
  Cc: Dmitry Vyukov, Andrew Morton, Linux-MM, Ingo Molnar, Will Deacon

Hello, Peter.

We got two reports. Neither RWSEM_READER_OWNED nor RWSEM_ANONYMOUSLY_OWNED is set, and
(presumably) sem->owner == current is true, but count is -1. What does this mean?

https://syzkaller.appspot.com/text?tag=CrashLog&x=169dbb9b400000

[ 2580.337550][ T3645] mmap_sem: hlock->read=1 count=-4294967295 current=ffff888050e04140, owner=ffff888050e04140
[ 2580.353526][ T3645] ------------[ cut here ]------------
[ 2580.367859][ T3645] downgrading a read lock
[ 2580.367935][ T3645] WARNING: CPU: 1 PID: 3645 at kernel/locking/lockdep.c:3572 lock_downgrade+0x35d/0xbe0
[ 2580.382206][ T3645] Kernel panic - not syncing: panic_on_warn set ...

https://syzkaller.appspot.com/text?tag=CrashLog&x=1542da4f400000

[  386.342585][T16698] mmap_sem: hlock->read=1 count=-4294967295 current=ffff8880512ae180, owner=ffff8880512ae180
[  386.348586][T16698] ------------[ cut here ]------------
[  386.357203][T16698] downgrading a read lock
[  386.357294][T16698] WARNING: CPU: 1 PID: 16698 at kernel/locking/lockdep.c:3572 lock_downgrade+0x35d/0xbe0
[  386.372148][T16698] Kernel panic - not syncing: panic_on_warn set ...

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] lockdep: Add debug printk() for downgrade_write() warning.
  2019-01-09 11:47       ` Tetsuo Handa
@ 2019-01-09 18:22         ` Waiman Long
  0 siblings, 0 replies; 6+ messages in thread
From: Waiman Long @ 2019-01-09 18:22 UTC (permalink / raw)
  To: Tetsuo Handa, Peter Zijlstra
  Cc: Dmitry Vyukov, Andrew Morton, Linux-MM, Ingo Molnar, Will Deacon

On 01/09/2019 06:47 AM, Tetsuo Handa wrote:
> Hello, Peter.
>
> We got two reports. Neither RWSEM_READER_OWNED nor RWSEM_ANONYMOUSLY_OWNED is set, and
> (presumably) sem->owner == current is true, but count is -1. What does this mean?
>
> https://syzkaller.appspot.com/text?tag=CrashLog&x=169dbb9b400000
>
> [ 2580.337550][ T3645] mmap_sem: hlock->read=1 count=-4294967295 current=ffff888050e04140, owner=ffff888050e04140
> [ 2580.353526][ T3645] ------------[ cut here ]------------
> [ 2580.367859][ T3645] downgrading a read lock
> [ 2580.367935][ T3645] WARNING: CPU: 1 PID: 3645 at kernel/locking/lockdep.c:3572 lock_downgrade+0x35d/0xbe0
> [ 2580.382206][ T3645] Kernel panic - not syncing: panic_on_warn set ...
>
> https://syzkaller.appspot.com/text?tag=CrashLog&x=1542da4f400000
>
> [  386.342585][T16698] mmap_sem: hlock->read=1 count=-4294967295 current=ffff8880512ae180, owner=ffff8880512ae180
> [  386.348586][T16698] ------------[ cut here ]------------
> [  386.357203][T16698] downgrading a read lock
> [  386.357294][T16698] WARNING: CPU: 1 PID: 16698 at kernel/locking/lockdep.c:3572 lock_downgrade+0x35d/0xbe0
> [  386.372148][T16698] Kernel panic - not syncing: panic_on_warn set ...
>
>
A call to up_read() while the count was previously 0 will cause the
count to become -1. I would suggest adding some debug code in up_read()
to catch the offending caller.

Cheers,
Longman

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH] lockdep: Add debug printk() for downgrade_write() warning.
  2019-01-07  5:58       ` Dmitry Vyukov
  (?)
  (?)
@ 2019-01-10 10:12       ` Tetsuo Handa
  -1 siblings, 0 replies; 6+ messages in thread
From: Tetsuo Handa @ 2019-01-10 10:12 UTC (permalink / raw)
  To: Dmitry Vyukov, Andrew Morton, Peter Zijlstra
  Cc: Linux-MM, Ingo Molnar, Will Deacon

On 2019/01/07 14:58, Dmitry Vyukov wrote:
> On Mon, Jan 7, 2019 at 5:19 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>> I tossed it in there.
>>
>> But I wonder if anyone is actually running this code.  Because
>>
>> --- a/lib/Kconfig.debug~info-task-hung-in-generic_file_write_iter
>> +++ a/lib/Kconfig.debug
>> @@ -2069,6 +2069,12 @@ config IO_STRICT_DEVMEM
>>
>>           If in doubt, say Y.
>>
>> +config DEBUG_AID_FOR_SYZBOT
>> +       bool "Additional debug code for syzbot"
>> +       default n
>> +       help
>> +         This option is intended for testing by syzbot.
>> +
> 
> 
> Yes, syzbot always defines this option:
> 
> https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kasan.config#L14
> https://github.com/google/syzkaller/blob/master/dashboard/config/upstream-kmsan.config#L9
> 
> It's meant specifically for such cases.
> 
> Tetsuo already got some useful information for past bugs using this feature.
> 

Andrew, you can drop this patch, for a patch that fixes this problem is already available
at https://lkml.kernel.org/r/1547093005-26085-1-git-send-email-longman@redhat.com .
Peter, please apply the fix from Waiman Long.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-01-10 10:12 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <1546771139-9349-1-git-send-email-penguin-kernel@I-love.SAKURA.ne.jp>
     [not found] ` <e1a38e21-d5fe-dee3-7081-bc1a12965a68@i-love.sakura.ne.jp>
2019-01-07  4:19   ` [PATCH] lockdep: Add debug printk() for downgrade_write() warning Andrew Morton
2019-01-07  5:58     ` Dmitry Vyukov
2019-01-07  5:58       ` Dmitry Vyukov
2019-01-09 11:47       ` Tetsuo Handa
2019-01-09 18:22         ` Waiman Long
2019-01-10 10:12       ` Tetsuo Handa

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.