linux-kernel - Re: [PATCH] ext4: Fix deadlock on page reclaim

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]

Message-ID: <20190726224423.GE7777@dread.disaster.area>
Date:   Sat, 27 Jul 2019 08:44:23 +1000
From:   Dave Chinner <david@...morbit.com>
To:     Christoph Hellwig <hch@...radead.org>
Cc:     Damien Le Moal <damien.lemoal@....com>,
        Theodore Ts'o <tytso@....edu>,
        Andreas Dilger <adilger.kernel@...ger.ca>,
        linux-ext4@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-fsdevel@...r.kernel.org, Christoph Hellwig <hch@....de>,
        Johannes Thumshirn <jthumshirn@...e.de>,
        Naohiro Aota <naohiro.aota@....com>,
        Masato Suzuki <masato.suzuki@....com>
Subject: Re: [PATCH] ext4: Fix deadlock on page reclaim

On Thu, Jul 25, 2019 at 04:54:42AM -0700, Christoph Hellwig wrote:
> On Thu, Jul 25, 2019 at 06:33:58PM +0900, Damien Le Moal wrote:
> > +	gfp_t gfp_mask;
> > +
> >  	switch (ext4_inode_journal_mode(inode)) {
> >  	case EXT4_INODE_ORDERED_DATA_MODE:
> >  	case EXT4_INODE_WRITEBACK_DATA_MODE:
> > @@ -4019,6 +4019,14 @@ void ext4_set_aops(struct inode *inode)
> >  		inode->i_mapping->a_ops = &ext4_da_aops;
> >  	else
> >  		inode->i_mapping->a_ops = &ext4_aops;
> > +
> > +	/*
> > +	 * Ensure all page cache allocations are done from GFP_NOFS context to
> > +	 * prevent direct reclaim recursion back into the filesystem and blowing
> > +	 * stacks or deadlocking.
> > +	 */
> > +	gfp_mask = mapping_gfp_mask(inode->i_mapping);
> > +	mapping_set_gfp_mask(inode->i_mapping, (gfp_mask & ~(__GFP_FS)));
> 
> This looks like something that could hit every file systems, so
> shouldn't we fix this in common code?  We could also look into
> just using memalloc_nofs_save for the page cache allocation path
> instead of the per-mapping gfp_mask.

I think it has to be the entire IO path - any allocation from the
underlying filesystem could recurse into the top level filesystem
and then deadlock if the memory reclaim submits IO or blocks on
IO completion from the upper filesystem. That's a bloody big hammer
for something that is only necessary when there are stacked
filesystems like this....

Cheers,

Dave.
-- 
Dave Chinner
david@...morbit.com