Re: [PATCH for 6.19-rc1] fs: preserve file type in make_bad_inode() unless invalid

[Tetsuo Handa]

On 2025/12/10 19:09, Jan Kara wrote:
> On Wed 10-12-25 18:45:26, Tetsuo Handa wrote:
>> syzbot is hitting VFS_BUG_ON_INODE(!S_ISDIR(inode->i_mode)) check
>> introduced by commit e631df89cd5d ("fs: speed up path lookup with cheaper
>> handling of MAY_EXEC"), for make_bad_inode() is blindly changing file type
>> to S_IFREG. Since make_bad_inode() might be called after an inode is fully
>> constructed, make_bad_inode() should not needlessly change file type.
>>
>> Reported-by: syzbot+d222f4b7129379c3d5bc@xxxxxxxxxxxxxxxxxxxxxxxxx
>> Closes: https://syzkaller.appspot.com/bug?extid=d222f4b7129379c3d5bc
>> Signed-off-by: Tetsuo Handa <penguin-kernel@xxxxxxxxxxxxxxxxxxx>
> 
> No. make_bad_inode() must not be called once the inode is fully visible
> because that can cause all sorts of fun. That function is really only good
> for handling a situation when read of an inode from the disk failed or
> similar early error paths.
I'm surprised to hear that.

But since commit 58b6fcd2ab34 ("ocfs2: mark inode bad upon validation failure
during read") is a bug fix, we want to somehow prevent this bug from re-opening.

Minimal change for this release cycle might look like

----------
diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
index b5fcc2725a29..2c97c8b4013f 100644
--- a/fs/ocfs2/inode.c
+++ b/fs/ocfs2/inode.c
@@ -1715,8 +1715,13 @@ int ocfs2_read_inode_block_full(struct inode *inode, struct buffer_head **bh,
 	rc = ocfs2_read_blocks(INODE_CACHE(inode), OCFS2_I(inode)->ip_blkno,
 			       1, &tmp, flags, ocfs2_validate_inode_block);
 
-	if (rc < 0)
+	if (rc < 0) {
+		/* Preserve file type while making operations no-op. */
+		umode_t	mode = inode->i_mode & S_IFMT;
+
 		make_bad_inode(inode);
+		inode->i_mode = mode;
+	}
 	/* If ocfs2_read_blocks() got us a new bh, pass it up. */
 	if (!rc && !*bh)
 		*bh = tmp;
----------

but what approach do you prefer?

Introduce a copy of bad_{inode,file}_ops for ocfs2 and replace
a call to make_bad_inode() with updating only {inode,file}_ops ?

Or, modify existing {inode,file}_ops for ocfs2 to check whether
an I/O error has occurred in the past?