LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Dave Chinner <david@fromorbit.com>
To: Eric Biggers <ebiggers@kernel.org>
Cc: linux-xfs@vger.kernel.org, linux-ext4@vger.kernel.org,
	syzkaller-bugs@googlegroups.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH] xfs: clear PF_MEMALLOC before exiting xfsaild thread
Date: Mon, 9 Mar 2020 10:03:07 +1100	[thread overview]
Message-ID: <20200308230307.GM10776@dread.disaster.area> (raw)
In-Reply-To: <20200308043540.1034779-1-ebiggers@kernel.org>

On Sat, Mar 07, 2020 at 08:35:40PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Leaving PF_MEMALLOC set when exiting a kthread causes it to remain set
> during do_exit().  That can confuse things.  For example, if BSD process
> accounting is enabled, then it's possible for do_exit() to end up
> calling ext4_write_inode().  That triggers the
> WARN_ON_ONCE(current->flags & PF_MEMALLOC) there, as it assumes
> (appropriately) that inodes aren't written when allocating memory.

And just how the hell does and XFS kernel thread end up calling
ext4_write_inode()? That's kinda a key factor in all this, and
it's not explained here.

> This case was reported by syzbot at
> https://lkml.kernel.org/r/0000000000000e7156059f751d7b@google.com.

Which doesn't really explain it, either.

What is the configuration conditions under which this triggers? It
looks like some weird combination of a no-journal ext4 root
filesystem and the audit subsystem being configured with O_SYNC
files?

People trying to decide if this is something that needs to be
backported to stable kernels need to be able to unerstand how this
bug is actually triggered so they can make sane decisions about
it...

/me tracks the PF_MEMALLOC flag back to commit 43ff2122e649 ("xfs:
on-stack delayed write buffer lists") where is was inherited here
from the buffer flush daemon that xfsaild took over from. Which also
never cleared the PF_MEMALLOC flag. That goes back to 2002:

commit d676c94914eb97d72061aff69c99406df4f395e9
Author: Steve Lord <lord@sgi.com>
Date:   Fri Jan 11 23:31:51 2002 +0000

    Merge pagebuf module into XFS

So this issue of calling do_exit() with PF_MEMALLOC set has been
around for 18+ years without anyone noticing it.

I also note that cifs_demultiplex_thread() has the same problem -
can you please do a complete audit of all the users of PF_MEMALLOC
and fix all of them?

> Fix this in xfsaild() by using the helper functions to save and restore
> PF_MEMALLOC.
> 
> Reported-by: syzbot+1f9dc49e8de2582d90c2@syzkaller.appspotmail.com
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
>  fs/xfs/xfs_trans_ail.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_trans_ail.c b/fs/xfs/xfs_trans_ail.c
> index 00cc5b8734be..3bc570c90ad9 100644
> --- a/fs/xfs/xfs_trans_ail.c
> +++ b/fs/xfs/xfs_trans_ail.c
> @@ -529,8 +529,9 @@ xfsaild(
>  {
>  	struct xfs_ail	*ailp = data;
>  	long		tout = 0;	/* milliseconds */
> +	unsigned int	noreclaim_flag;
>  
> -	current->flags |= PF_MEMALLOC;
> +	noreclaim_flag = memalloc_noreclaim_save();
>  	set_freezable();
>  
>  	while (1) {
> @@ -601,6 +602,7 @@ xfsaild(
>  		tout = xfsaild_push(ailp);
>  	}
>  
> +	memalloc_noreclaim_restore(noreclaim_flag);
>  	return 0;
>  }

The code looks fine - I considered doing this a couple of weeks ago
just for cleaniness reasons - but the commit message needs work to
explain the context of the bug...

Cheers,

Dave.
-- 
Dave Chinner
david@fromorbit.com

  reply	other threads:[~2020-03-08 23:03 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-02-26  6:57 WARNING in ext4_write_inode syzbot
2020-03-08  4:35 ` [PATCH] xfs: clear PF_MEMALLOC before exiting xfsaild thread Eric Biggers
2020-03-08 23:03   ` Dave Chinner [this message]
2020-03-09  1:04     ` Eric Biggers
2020-03-09  4:34       ` [PATCH v2] " Eric Biggers
2020-03-09 10:57         ` Brian Foster
2020-03-09 16:24         ` Darrick J. Wong
2020-03-09 18:04           ` Eric Biggers
2020-03-09 18:13             ` Darrick J. Wong
2020-03-09 18:57               ` [PATCH v3] " Eric Biggers
2020-03-10 15:47                 ` Darrick J. Wong
2020-03-11  6:34                 ` Christoph Hellwig
2020-03-12 22:20           ` [PATCH v2] " Eric Biggers
2020-03-08  4:36 ` [PATCH] cifs: clear PF_MEMALLOC before exiting demultiplex thread Eric Biggers
2020-03-08  6:16   ` [PATCH v2] " Eric Biggers
2020-03-08 18:43     ` Steve French
2020-03-09  5:56       ` Eric Biggers
2020-03-09  5:58         ` [PATCH v3] " Eric Biggers

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200308230307.GM10776@dread.disaster.area \
    --to=david@fromorbit.com \
    --cc=ebiggers@kernel.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=syzkaller-bugs@googlegroups.com \
    --subject='Re: [PATCH] xfs: clear PF_MEMALLOC before exiting xfsaild thread' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).