lkml.org 
[lkml]   [2017]   [Nov]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
From
Date
SubjectRe: WARNING in lock_release
On Fri, Nov 17, 2017 at 10:02 PM, Al Viro <viro@zeniv.linux.org.uk> wrote:
> On Thu, Nov 16, 2017 at 02:56:00AM -0800, syzbot wrote:
>> Hello,
>>
>> syzkaller hit the following crash on
>> 5515cf16e270538121e4fa9283fed86c6cfd8c9c
>> git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/master
>> compiler: gcc (GCC) 7.1.1 20170620
>> .config is attached
>> Raw console output is attached.
>> C reproducer is attached
>> syzkaller reproducer is attached. See https://goo.gl/kgGztJ
>> for information about syzkaller reproducers
>
> Hmm... That's alloc_super() buggering off on allocation failure and
> hitting up_write(s->s_umount) in destroy_unused_super(), since it has
> not done
> init_rwsem(&s->s_umount);
> lockdep_set_class(&s->s_umount, &type->s_umount_key);
> down_write_nested(&s->s_umount, SINGLE_DEPTH_NESTING);
> part yet. The sucker is just all-zeroes here. The easiest way to fix
> that would probably be to move that bit of initialization in the very
> beginning...
>
> diff --git a/fs/super.c b/fs/super.c
> index 8ca15415351a..2808aeaf5337 100644
> --- a/fs/super.c
> +++ b/fs/super.c
> @@ -190,6 +190,24 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
>
> INIT_LIST_HEAD(&s->s_mounts);
> s->s_user_ns = get_user_ns(user_ns);
> + init_rwsem(&s->s_umount);
> + lockdep_set_class(&s->s_umount, &type->s_umount_key);
> + /*
> + * sget() can have s_umount recursion.
> + *
> + * When it cannot find a suitable sb, it allocates a new
> + * one (this one), and tries again to find a suitable old
> + * one.
> + *
> + * In case that succeeds, it will acquire the s_umount
> + * lock of the old one. Since these are clearly distrinct
> + * locks, and this object isn't exposed yet, there's no
> + * risk of deadlocks.
> + *
> + * Annotate this by putting this lock in a different
> + * subclass.
> + */
> + down_write_nested(&s->s_umount, SINGLE_DEPTH_NESTING);
>
> if (security_sb_alloc(s))
> goto fail;
> @@ -217,25 +235,6 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
> goto fail;
> if (list_lru_init_memcg(&s->s_inode_lru))
> goto fail;
> -
> - init_rwsem(&s->s_umount);
> - lockdep_set_class(&s->s_umount, &type->s_umount_key);
> - /*
> - * sget() can have s_umount recursion.
> - *
> - * When it cannot find a suitable sb, it allocates a new
> - * one (this one), and tries again to find a suitable old
> - * one.
> - *
> - * In case that succeeds, it will acquire the s_umount
> - * lock of the old one. Since these are clearly distrinct
> - * locks, and this object isn't exposed yet, there's no
> - * risk of deadlocks.
> - *
> - * Annotate this by putting this lock in a different
> - * subclass.
> - */
> - down_write_nested(&s->s_umount, SINGLE_DEPTH_NESTING);
> s->s_count = 1;
> atomic_set(&s->s_active, 1);
> mutex_init(&s->s_vfs_rename_mutex);


Hi,

We are rolling out patch testing feature for syzbot:
https://github.com/google/syzkaller/blob/master/docs/syzbot.md#communication-with-syzbot
So if you are asking for testing, feel free to use it. If not, let's
still give it a try (it also needs testing):

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
diff --git a/fs/super.c b/fs/super.c
index 8ca15415351a..2808aeaf5337 100644
--- a/fs/super.c
+++ b/fs/super.c
@@ -190,6 +190,24 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,

INIT_LIST_HEAD(&s->s_mounts);
s->s_user_ns = get_user_ns(user_ns);
+ init_rwsem(&s->s_umount);
+ lockdep_set_class(&s->s_umount, &type->s_umount_key);
+ /*
+ * sget() can have s_umount recursion.
+ *
+ * When it cannot find a suitable sb, it allocates a new
+ * one (this one), and tries again to find a suitable old
+ * one.
+ *
+ * In case that succeeds, it will acquire the s_umount
+ * lock of the old one. Since these are clearly distrinct
+ * locks, and this object isn't exposed yet, there's no
+ * risk of deadlocks.
+ *
+ * Annotate this by putting this lock in a different
+ * subclass.
+ */
+ down_write_nested(&s->s_umount, SINGLE_DEPTH_NESTING);

if (security_sb_alloc(s))
goto fail;
@@ -217,25 +235,6 @@ static struct super_block *alloc_super(struct file_system_type *type, int flags,
goto fail;
if (list_lru_init_memcg(&s->s_inode_lru))
goto fail;
-
- init_rwsem(&s->s_umount);
- lockdep_set_class(&s->s_umount, &type->s_umount_key);
- /*
- * sget() can have s_umount recursion.
- *
- * When it cannot find a suitable sb, it allocates a new
- * one (this one), and tries again to find a suitable old
- * one.
- *
- * In case that succeeds, it will acquire the s_umount
- * lock of the old one. Since these are clearly distrinct
- * locks, and this object isn't exposed yet, there's no
- * risk of deadlocks.
- *
- * Annotate this by putting this lock in a different
- * subclass.
- */
- down_write_nested(&s->s_umount, SINGLE_DEPTH_NESTING);
s->s_count = 1;
atomic_set(&s->s_active, 1);
mutex_init(&s->s_vfs_rename_mutex);
\
 
 \ /
  Last update: 2017-11-20 14:06    [W:0.044 / U:0.824 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site