[PATCH 5.10/5.15/6.1 0/1] nilfs2: fix WARNING in nilfs_dat_prepare_end()

[PATCH 5.10/5.15/6.1 0/1] nilfs2: fix WARNING in nilfs_dat_prepare_end()

[Roman Smirnov]

Syzkaller reports WARNING in nilfs_dat_prepare_end() in 5.10, 5.15 and 6.1
stable releases. The problem has been fixed in upstream:
https://syzkaller.appspot.com/bug?extid=5d5d25f90f195a3cfcb4

The problem can also be fixed in versions 5.10, 5.15 and 6.1 by the
following patch.

Found by Linux Verification Center (linuxtesting.org) with Syzkaller.

Link: https://syzkaller.appspot.com/bug?extid=325e6b0a1e7cf9035cc0
Link: https://syzkaller.appspot.com/bug?extid=bebf30d67ea2569f0fd3

Ryusuke Konishi (1):
  nilfs2: replace WARN_ONs for invalid DAT metadata block requests

 fs/nilfs2/dat.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

-- 
2.34.1

[PATCH 5.10/5.15/6.1 1/1] nilfs2: replace WARN_ONs for invalid DAT metadata block requests

[Roman Smirnov]

From: Ryusuke Konishi <konishi.ryusuke@gmail.com>

commit 5124a0a549857c4b87173280e192eea24dea72ad upstream.

If DAT metadata file block access fails due to corruption of the DAT file
or abnormal virtual block numbers held by b-trees or inodes, a kernel
warning is generated.

This replaces the WARN_ONs by error output, so that a kernel, booted with
panic_on_warn, does not panic.  This patch also replaces the detected
return code -ENOENT with another internal code -EINVAL to notify the bmap
layer of metadata corruption.  When the bmap layer sees -EINVAL, it
handles the abnormal situation with nilfs_bmap_convert_error() and finally
returns code -EIO as it should.

Link: https://lkml.kernel.org/r/0000000000005cc3d205ea23ddcf@google.com
Link: https://lkml.kernel.org/r/20230126164114.6911-1-konishi.ryusuke@gmail.com
Signed-off-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
Reported-by: <syzbot+5d5d25f90f195a3cfcb4@syzkaller.appspotmail.com>
Tested-by: Ryusuke Konishi <konishi.ryusuke@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Roman Smirnov <r.smirnov@omp.ru>
---
 fs/nilfs2/dat.c | 27 +++++++++++++++++----------
 1 file changed, 17 insertions(+), 10 deletions(-)

diff --git a/fs/nilfs2/dat.c b/fs/nilfs2/dat.c
index 9930fa901039..1e7f653c1df7 100644
--- a/fs/nilfs2/dat.c
+++ b/fs/nilfs2/dat.c
@@ -40,8 +40,21 @@ static inline struct nilfs_dat_info *NILFS_DAT_I(struct inode *dat)
 static int nilfs_dat_prepare_entry(struct inode *dat,
 				   struct nilfs_palloc_req *req, int create)
 {
-	return nilfs_palloc_get_entry_block(dat, req->pr_entry_nr,
-					    create, &req->pr_entry_bh);
+	int ret;
+
+	ret = nilfs_palloc_get_entry_block(dat, req->pr_entry_nr,
+					   create, &req->pr_entry_bh);
+	if (unlikely(ret == -ENOENT)) {
+		nilfs_err(dat->i_sb,
+			  "DAT doesn't have a block to manage vblocknr = %llu",
+			  (unsigned long long)req->pr_entry_nr);
+		/*
+		 * Return internal code -EINVAL to notify bmap layer of
+		 * metadata corruption.
+		 */
+		ret = -EINVAL;
+	}
+	return ret;
 }
 
 static void nilfs_dat_commit_entry(struct inode *dat,
@@ -123,11 +136,7 @@ static void nilfs_dat_commit_free(struct inode *dat,
 
 int nilfs_dat_prepare_start(struct inode *dat, struct nilfs_palloc_req *req)
 {
-	int ret;
-
-	ret = nilfs_dat_prepare_entry(dat, req, 0);
-	WARN_ON(ret == -ENOENT);
-	return ret;
+	return nilfs_dat_prepare_entry(dat, req, 0);
 }
 
 void nilfs_dat_commit_start(struct inode *dat, struct nilfs_palloc_req *req,
@@ -154,10 +163,8 @@ int nilfs_dat_prepare_end(struct inode *dat, struct nilfs_palloc_req *req)
 	int ret;
 
 	ret = nilfs_dat_prepare_entry(dat, req, 0);
-	if (ret < 0) {
-		WARN_ON(ret == -ENOENT);
+	if (ret < 0)
 		return ret;
-	}
 
 	kaddr = kmap_atomic(req->pr_entry_bh->b_page);
 	entry = nilfs_palloc_block_get_entry(dat, req->pr_entry_nr,
-- 
2.34.1

Re: [PATCH 5.10/5.15/6.1 0/1] nilfs2: fix WARNING in nilfs_dat_prepare_end()

[Roman Smirnov]

On Thu, 8 Feb 2024 17:42:41 +0300, Roman Smirnov wrote:
> Syzkaller reports WARNING in nilfs_dat_prepare_end() in 5.10, 5.15 and 6.1
> stable releases. The problem has been fixed in upstream:
> https://syzkaller.appspot.com/bug?extid=5d5d25f90f195a3cfcb4
>
> The problem can also be fixed in versions 5.10, 5.15 and 6.1 by the
> following patch.
>
> Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
> 
> Link: https://syzkaller.appspot.com/bug?extid=325e6b0a1e7cf9035cc0
> Link: https://syzkaller.appspot.com/bug?extid=bebf30d67ea2569f0fd3
> 
> Ryusuke Konishi (1):
>  nilfs2: replace WARN_ONs for invalid DAT metadata block requests
>
>  fs/nilfs2/dat.c | 27 +++++++++++++++++----------
>  1 file changed, 17 insertions(+), 10 deletions(-)

Sorry to bother you, do you have any comments on the patch?

Re: [PATCH 5.10/5.15/6.1 0/1] nilfs2: fix WARNING in nilfs_dat_prepare_end()

[Ryusuke Konishi]

On Tue, Feb 20, 2024 at 5:44 PM Roman Smirnov wrote:
>
> On Thu, 8 Feb 2024 17:42:41 +0300, Roman Smirnov wrote:
> > Syzkaller reports WARNING in nilfs_dat_prepare_end() in 5.10, 5.15 and 6.1
> > stable releases. The problem has been fixed in upstream:
> > https://syzkaller.appspot.com/bug?extid=5d5d25f90f195a3cfcb4
> >
> > The problem can also be fixed in versions 5.10, 5.15 and 6.1 by the
> > following patch.
> >
> > Found by Linux Verification Center (linuxtesting.org) with Syzkaller.
> >
> > Link: https://syzkaller.appspot.com/bug?extid=325e6b0a1e7cf9035cc0
> > Link: https://syzkaller.appspot.com/bug?extid=bebf30d67ea2569f0fd3
> >
> > Ryusuke Konishi (1):
> >  nilfs2: replace WARN_ONs for invalid DAT metadata block requests
> >
> >  fs/nilfs2/dat.c | 27 +++++++++++++++++----------
> >  1 file changed, 17 insertions(+), 10 deletions(-)
>
> Sorry to bother you, do you have any comments on the patch?

Hi Greg,

As a side note, this commit handles certain metadata corruptions
better if they are detected, rather than just killing WARN_ONs, and
prevents an internal error code (ENOENT) from propagating
inappropriately to userland.

So, in retrospect, I think it was worth backporting to stable trees.

I have checked the source code of each of the target stable trees, and
they are safe to apply.

Thanks,
Ryusuke Konishi