linux-kernel - Re: [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read

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: <CAHxJ8O_7-PfJRyGp9-1KOkwmYJWQDzCvvo_P-jxzbzHoqXyH9Q@mail.gmail.com>
Date: Fri, 31 Oct 2025 01:07:56 +0300
From: Ahmet Eray Karadag <eraykrdg1@...il.com>
To: Heming Zhao <heming.zhao@...e.com>
Cc: mark@...heh.com, jlbec@...lplan.org, joseph.qi@...ux.alibaba.com, 
	ocfs2-devel@...ts.linux.dev, linux-kernel@...r.kernel.org, 
	david.hunter.linux@...il.com, skhan@...uxfoundation.org, 
	syzbot+b93b65ee321c97861072@...kaller.appspotmail.com, 
	Albin Babu Varghese <albinbabuvarghese20@...il.com>
Subject: Re: [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure
 during read

Hi Heming,

Thanks a lot for the review and comments.

Regarding the placement of make_bad_inode(), our patch places it in
ocfs2_read_inode_block_full() because
ocfs2_validate_inode_block() itself doesn't have access
to the inode object. I believe this (in the caller) is the
correct layer, as it seems to match other patterns in OCFS2
where the caller handles the make_bad_inode upon validation
failure.

I had one question about your proposal to combine this patch with
Albin's [1]. When you mentioned, "We should forbid any write
operations," were you referring to Albin's read-only check in
ocfs2_setattr as the mechanism to "forbid" the operation? Or
were you suggesting we should use the inode size sanity check
itself (e.g., by converting the BUG_ON to an -EIO return)
as that mechanism?

Thanks,
Eray

Heming Zhao <heming.zhao@...e.com>, 30 Eki 2025 Per, 10:59 tarihinde şunu yazdı:
>
> Hi,
>
> In my view, combining this patch and another patch [1] is a complete
> solution for this bug.
>
> According to the oops stack, the FS is already in read-only mode.
> We should forbid any write operations and then perform the inode
> sanity check.
>
> And I think ocfs2_validate_inode_block is a good place for make_bad_inode().
>
> [1]:
> https://syzkaller.appspot.com/text?tag=Patch&x=1287f614580000
> - by albinbabuvarghese20@...il.com from:
>   https://syzkaller.appspot.com/bug?extid=b93b65ee321c97861072
>
> Thanks,
> Heming
>
> On Thu, Oct 30, 2025 at 01:57:49AM +0300, Ahmet Eray Karadag wrote:
> > Potentially triggered by sequences like buffered writes followed by
> > open(O_DIRECT), can result in an invalid on-disk inode block
> > (e.g., bad signature). OCFS2 detects this corruption when reading the
> > inode block via ocfs2_validate_inode_block(), logs "Invalid dinode",
> > and often switches the filesystem to read-only mode.
> >
> > Currently, the function reading the inode block (ocfs2_read_inode_block_full())
> > fails to call make_bad_inode() upon detecting the validation error.
> > Because the in-memory inode is not marked bad, subsequent operations
> > (like ftruncate) proceed erroneously. They eventually reach code
> > (e.g., ocfs2_truncate_file()) that compares the inconsistent
> > in-memory size (38639) against the invalid/stale on-disk size (0), leading
> > to kernel crashes via BUG_ON.
> >
> > Fix this by calling make_bad_inode(inode) within the error handling path of
> > ocfs2_read_inode_block_full() immediately after a block read or validation
> > error occurs. This ensures VFS is properly notified about the
> > corrupt inode at the point of detection. Marking the inode bad  allows VFS
> > to correctly fail subsequent operations targeting this inode early,
> > preventing kernel panics caused by operating on known inconsistent inode states.
> >
> > [RFC]: While this patch prevents the kernel crash triggered by the reproducer,
> > feedback is requested on whether ocfs2_read_inode_block_full() is the most
> > appropriate layer to call make_bad_inode(). Should this check perhaps reside
> > within the caller or should the error propagation be handled differently?:
> > Input on the best practice for handling this specific VFS inconsistency
> > within OCFS2 would be appreciated.
> >
> > Reported-by: syzbot+b93b65ee321c97861072@...kaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/bug?extid=b93b65ee321c97861072
> > Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@...il.com>
> > Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@...il.com>
> > Signed-off-by: Ahmet Eray Karadag <eraykrdg1@...il.com>
> > ---
> >  fs/ocfs2/inode.c | 2 ++
> >  1 file changed, 2 insertions(+)
> >
> > diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c
> > index fcc89856ab95..415ad29ec758 100644
> > --- a/fs/ocfs2/inode.c
> > +++ b/fs/ocfs2/inode.c
> > @@ -1690,6 +1690,8 @@ 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)
> > +             make_bad_inode(inode);
> >       /* If ocfs2_read_blocks() got us a new bh, pass it up. */
> >       if (!rc && !*bh)
> >               *bh = tmp;
> > --
> > 2.43.0
> >
> >