linux-kernel - [PATCH 2/3] nilfs2: add missing check for inode numbers on directory entries

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: <20240623051135.4180-3-konishi.ryusuke@gmail.com>
Date: Sun, 23 Jun 2024 14:11:34 +0900
From: Ryusuke Konishi <konishi.ryusuke@...il.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-nilfs <linux-nilfs@...r.kernel.org>,
	syzbot <syzbot+d79afb004be235636ee8@...kaller.appspotmail.com>,
	syzkaller-bugs@...glegroups.com,
	LKML <linux-kernel@...r.kernel.org>,
	hdanton@...a.com,
	jack@...e.cz,
	linux-fsdevel@...r.kernel.org,
	willy@...radead.org
Subject: [PATCH 2/3] nilfs2: add missing check for inode numbers on directory entries

Syzbot reported that mounting and unmounting a specific pattern of
corrupted nilfs2 filesystem images causes a use-after-free of metadata
file inodes, which triggers a kernel bug in lru_add_fn().

As Jan Kara pointed out, this is because the link count of a metadata
file gets corrupted to 0, and nilfs_evict_inode(), which is called
from iput(), tries to delete that inode (ifile inode in this case).

The inconsistency occurs because directories containing the inode
numbers of these metadata files that should not be visible in the
namespace are read without checking.

Fix this issue by treating the inode numbers of these internal files
as errors in the sanity check helper when reading directory
folios/pages.

Also thanks to Hillf Danton and Matthew Wilcox for their initial
mm-layer analysis.

Signed-off-by: Ryusuke Konishi <konishi.ryusuke@...il.com>
Reported-by: syzbot+d79afb004be235636ee8@...kaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=d79afb004be235636ee8
Reported-by: Jan Kara <jack@...e.cz>
Closes: https://lkml.kernel.org/r/20240617075758.wewhukbrjod5fp5o@quack3
Tested-by: Ryusuke Konishi <konishi.ryusuke@...il.com>
Cc: stable@...r.kernel.org
---
 fs/nilfs2/dir.c   | 6 ++++++
 fs/nilfs2/nilfs.h | 5 +++++
 2 files changed, 11 insertions(+)

diff --git a/fs/nilfs2/dir.c b/fs/nilfs2/dir.c
index 52e50b1b7f22..dddfa604491a 100644
--- a/fs/nilfs2/dir.c
+++ b/fs/nilfs2/dir.c
@@ -135,6 +135,9 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
 			goto Enamelen;
 		if (((offs + rec_len - 1) ^ offs) & ~(chunk_size-1))
 			goto Espan;
+		if (unlikely(p->inode &&
+			     NILFS_PRIVATE_INODE(le64_to_cpu(p->inode))))
+			goto Einumber;
 	}
 	if (offs != limit)
 		goto Eend;
@@ -160,6 +163,9 @@ static bool nilfs_check_folio(struct folio *folio, char *kaddr)
 	goto bad_entry;
 Espan:
 	error = "directory entry across blocks";
+	goto bad_entry;
+Einumber:
+	error = "disallowed inode number";
 bad_entry:
 	nilfs_error(sb,
 		    "bad entry in directory #%lu: %s - offset=%lu, inode=%lu, rec_len=%zd, name_len=%d",
diff --git a/fs/nilfs2/nilfs.h b/fs/nilfs2/nilfs.h
index 7e39e277c77f..4017f7856440 100644
--- a/fs/nilfs2/nilfs.h
+++ b/fs/nilfs2/nilfs.h
@@ -121,6 +121,11 @@ enum {
 	((ino) >= NILFS_FIRST_INO(sb) ||				\
 	 ((ino) < NILFS_USER_INO && (NILFS_SYS_INO_BITS & BIT(ino))))
 
+#define NILFS_PRIVATE_INODE(ino) ({					\
+	ino_t __ino = (ino);						\
+	((__ino) < NILFS_USER_INO && (__ino) != NILFS_ROOT_INO &&	\
+	 (__ino) != NILFS_SKETCH_INO); })
+
 /**
  * struct nilfs_transaction_info: context information for synchronization
  * @ti_magic: Magic number
-- 
2.34.1