Received-SPF: pass (google.com: domain of linux-kernel+bounces-216012-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45d1:ec00::1 as permitted sender) client-ip=2604:1380:45d1:ec00::1;
Date: Sat, 15 Jun 2024 21:44:54 +0100
From: Matthew Wilcox <willy@infradead.org>
To: Hillf Danton <hdanton@sina.com>
Cc: linux-mm@kvack.org, Jan Kara <jack@suse.cz>,
	linux-kernel@vger.kernel.org,
	syzbot+d79afb004be235636ee8@syzkaller.appspotmail.com,
	linux-fsdevel@vger.kernel.org, linux-nilfs@vger.kernel.org,
	Ryusuke Konishi <konishi.ryusuke@gmail.com>
Subject: Re: [RFC PATCH] mm: truncate: flush lru cache for evicted inode
Message-ID: <Zm39RkZMjHdui8nh@casper.infradead.org>
References: <ZmxIvIJ3YSZDwbPW@casper.infradead.org>
 <20240614235953.809-1-hdanton@sina.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20240614235953.809-1-hdanton@sina.com>

On Sat, Jun 15, 2024 at 07:59:53AM +0800, Hillf Danton wrote:
> On Fri, 14 Jun 2024 14:42:20 +0100 Matthew Wilcox wrote:
> > On Fri, Jun 14, 2024 at 09:18:56PM +0800, Hillf Danton wrote:
> > > Flush lru cache to avoid folio->mapping uaf in case of inode teardown.
> > 
> > What?  inodes are supposed to have all their folios removed before
> > being freed.  Part of removing a folio sets the folio->mapping to NULL.
> > Where is the report?
> >
> Subject: Re: [syzbot] [nilfs?] [mm?] KASAN: slab-use-after-free Read in lru_add_fn
> https://lore.kernel.org/lkml/000000000000cae276061aa12d5e@google.com/

Thanks.  This fix is wrong.  Of course syzbot says it fixes the problem,
but you're just avoiding putting the folios into the situation where we
have debug that would detect the problem.

I suspect this would trigger:

+++ b/fs/inode.c
@@ -282,6 +282,7 @@ static struct inode *alloc_inode(struct super_block *sb)
 void __destroy_inode(struct inode *inode)
 {
        BUG_ON(inode_has_buffers(inode));
+       BUG_ON(inode->i_data.nrpages);
        inode_detach_wb(inode);
        security_inode_free(inode);
        fsnotify_inode_delete(inode);

and what a real fix would look like would be calling clear_inode()
before calling iput() in nilfs_put_root().  But I'm not an expert
in this layer of the VFS, so I might well be wrong.