public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read
@ 2025-10-29 22:57 Ahmet Eray Karadag
  2025-10-30  7:59 ` Heming Zhao
  2025-10-31  7:10 ` Heming Zhao
  0 siblings, 2 replies; 12+ messages in thread
From: Ahmet Eray Karadag @ 2025-10-29 22:57 UTC (permalink / raw)
  To: mark, jlbec, joseph.qi
  Cc: ocfs2-devel, linux-kernel, david.hunter.linux, skhan,
	Ahmet Eray Karadag, syzbot+b93b65ee321c97861072,
	Albin Babu Varghese

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@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=b93b65ee321c97861072
Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.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


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read
  2025-10-29 22:57 [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read Ahmet Eray Karadag
@ 2025-10-30  7:59 ` Heming Zhao
  2025-10-30 22:07   ` Ahmet Eray Karadag
  2025-10-31  7:10 ` Heming Zhao
  1 sibling, 1 reply; 12+ messages in thread
From: Heming Zhao @ 2025-10-30  7:59 UTC (permalink / raw)
  To: Ahmet Eray Karadag
  Cc: mark, jlbec, joseph.qi, ocfs2-devel, linux-kernel,
	david.hunter.linux, skhan, syzbot+b93b65ee321c97861072,
	Albin Babu Varghese

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@gmail.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@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?extid=b93b65ee321c97861072
> Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.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
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read
  2025-10-30  7:59 ` Heming Zhao
@ 2025-10-30 22:07   ` Ahmet Eray Karadag
  2025-10-31  2:30     ` Heming Zhao
  0 siblings, 1 reply; 12+ messages in thread
From: Ahmet Eray Karadag @ 2025-10-30 22:07 UTC (permalink / raw)
  To: Heming Zhao
  Cc: mark, jlbec, joseph.qi, ocfs2-devel, linux-kernel,
	david.hunter.linux, skhan, syzbot+b93b65ee321c97861072,
	Albin Babu Varghese

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@suse.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@gmail.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@syzkaller.appspotmail.com
> > Link: https://syzkaller.appspot.com/bug?extid=b93b65ee321c97861072
> > Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> > Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> > Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.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
> >
> >

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read
  2025-10-30 22:07   ` Ahmet Eray Karadag
@ 2025-10-31  2:30     ` Heming Zhao
  2025-10-31  3:59       ` Albin Babu Varghese
  0 siblings, 1 reply; 12+ messages in thread
From: Heming Zhao @ 2025-10-31  2:30 UTC (permalink / raw)
  To: Ahmet Eray Karadag
  Cc: mark, jlbec, joseph.qi, ocfs2-devel, linux-kernel,
	david.hunter.linux, skhan, syzbot+b93b65ee321c97861072,
	Albin Babu Varghese

On Fri, Oct 31, 2025 at 01:07:56AM +0300, Ahmet Eray Karadag wrote:
> 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.

Thanks for pointing that out. I agree with the above comments.
> 
> 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

The 'forbid' refers to the read-only check in ocfs2_setattr.
We can refer to ext4_setattr(), which calls ext4_emergency_state()
to forbid write operations.

- Heming
> 
> Heming Zhao <heming.zhao@suse.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@gmail.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@syzkaller.appspotmail.com
> > > Link: https://syzkaller.appspot.com/bug?extid=b93b65ee321c97861072
> > > Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> > > Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> > > Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.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
> > >
> > >

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read
  2025-10-31  2:30     ` Heming Zhao
@ 2025-10-31  3:59       ` Albin Babu Varghese
  2025-10-31  7:04         ` Heming Zhao
  0 siblings, 1 reply; 12+ messages in thread
From: Albin Babu Varghese @ 2025-10-31  3:59 UTC (permalink / raw)
  To: Heming Zhao
  Cc: Ahmet Eray Karadag, mark, jlbec, joseph.qi, ocfs2-devel,
	linux-kernel, david.hunter.linux, skhan,
	syzbot+b93b65ee321c97861072

Hi Heming, Thanks for the feedback.

> > 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?
> > 
> 
> The 'forbid' refers to the read-only check in ocfs2_setattr.
> We can refer to ext4_setattr(), which calls ext4_emergency_state()
> to forbid write operations.

I just looked at the ext4 implementation you mentioned. When we were working on
it, I actually referenced how XFS's setattr was handling this because I
couldn't find the exact ext4 implementation for this at the time, so I wasn't
sure. From what I understand now, ext4 is doing something similar too.

If everything looks good, we can combine these two patches and send them as a
patch series.

Best,
	Albin

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read
  2025-10-31  3:59       ` Albin Babu Varghese
@ 2025-10-31  7:04         ` Heming Zhao
  2025-10-31 13:54           ` Albin Babu Varghese
  0 siblings, 1 reply; 12+ messages in thread
From: Heming Zhao @ 2025-10-31  7:04 UTC (permalink / raw)
  To: Albin Babu Varghese
  Cc: Ahmet Eray Karadag, mark, jlbec, joseph.qi, ocfs2-devel,
	linux-kernel, david.hunter.linux, skhan,
	syzbot+b93b65ee321c97861072

On Thu, Oct 30, 2025 at 11:59:08PM -0400, Albin Babu Varghese wrote:
> Hi Heming, Thanks for the feedback.
> 
> > > 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?
> > > 
> > 
> > The 'forbid' refers to the read-only check in ocfs2_setattr.
> > We can refer to ext4_setattr(), which calls ext4_emergency_state()
> > to forbid write operations.
> 
> I just looked at the ext4 implementation you mentioned. When we were working on
> it, I actually referenced how XFS's setattr was handling this because I
> couldn't find the exact ext4 implementation for this at the time, so I wasn't
> sure. From what I understand now, ext4 is doing something similar too.
> 
> If everything looks good, we can combine these two patches and send them as a
> patch series.
> 
> Best,
> 	Albin

I support adding make_bad_inode() in ocfs2_read_inode_block_full().
ocfs2_read_locked_inode() calls ocfs2_read_inode_block[_full] to read the inode
from disk. However, ocfs2_read_inode_block[_full] have many callers, and in
current code, only ocfs2_read_locked_inode() marks the inode as bad. All others
forget to set the bad_inode.

The 'forbid' write operations when read-only mode is worth another patch, and
I plan to create this patch. This patch adds a similar ext4_emergency_state()
function for ocfs2.
Therefore, your original patch looks good to me. I will provide my Reivewed-by
for it.

Thanks,
Heming

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read
  2025-10-29 22:57 [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read Ahmet Eray Karadag
  2025-10-30  7:59 ` Heming Zhao
@ 2025-10-31  7:10 ` Heming Zhao
  1 sibling, 0 replies; 12+ messages in thread
From: Heming Zhao @ 2025-10-31  7:10 UTC (permalink / raw)
  To: Ahmet Eray Karadag
  Cc: mark, jlbec, joseph.qi, ocfs2-devel, linux-kernel,
	david.hunter.linux, skhan, syzbot+b93b65ee321c97861072,
	Albin Babu Varghese

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@syzkaller.appspotmail.com
> Link: https://syzkaller.appspot.com/bug?extid=b93b65ee321c97861072
> Co-developed-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Albin Babu Varghese <albinbabuvarghese20@gmail.com>
> Signed-off-by: Ahmet Eray Karadag <eraykrdg1@gmail.com>

LGTM.
There are some review comments in this mail thread; anyone who is interested
in them can check them:
https://lore.kernel.org/all/20251029225748.11361-2-eraykrdg1@gmail.com/T/

Reviewed-by: Heming Zhao <heming.zhao@suse.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
> 
> 

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read
  2025-10-31  7:04         ` Heming Zhao
@ 2025-10-31 13:54           ` Albin Babu Varghese
  2025-10-31 14:05             ` Heming Zhao
  0 siblings, 1 reply; 12+ messages in thread
From: Albin Babu Varghese @ 2025-10-31 13:54 UTC (permalink / raw)
  To: Heming Zhao
  Cc: Ahmet Eray Karadag, mark, jlbec, joseph.qi, ocfs2-devel,
	linux-kernel, david.hunter.linux, skhan,
	syzbot+b93b65ee321c97861072

Hi Heming,

> I support adding make_bad_inode() in ocfs2_read_inode_block_full().
> ocfs2_read_locked_inode() calls ocfs2_read_inode_block[_full] to read the inode
> from disk. However, ocfs2_read_inode_block[_full] have many callers, and in
> current code, only ocfs2_read_locked_inode() marks the inode as bad. All others
> forget to set the bad_inode.
> 
> The 'forbid' write operations when read-only mode is worth another patch, and
> I plan to create this patch. This patch adds a similar ext4_emergency_state()
> function for ocfs2.

We're working on this as part of the Linux Kernel Mentorship Program, and we'd
love to take on implementing the read-only check if it's not overly
complicated. We're just beginners, but we thought it would be a great learning
experience to work on this following the ext4 pattern you mentioned - if you
haven't already started working on it by the time you see this reply.

> Therefore, your original patch looks good to me. I will provide my Reivewed-by
> for it.

Thank you so much for your review and the Reviewed-by tag

Cheers,
	Albin

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read
  2025-10-31 13:54           ` Albin Babu Varghese
@ 2025-10-31 14:05             ` Heming Zhao
  2025-10-31 14:22               ` Albin Babu Varghese
  0 siblings, 1 reply; 12+ messages in thread
From: Heming Zhao @ 2025-10-31 14:05 UTC (permalink / raw)
  To: Albin Babu Varghese
  Cc: Ahmet Eray Karadag, mark, jlbec, joseph.qi, ocfs2-devel,
	linux-kernel, david.hunter.linux, skhan,
	syzbot+b93b65ee321c97861072

On Fri, Oct 31, 2025 at 09:54:41AM -0400, Albin Babu Varghese wrote:
> Hi Heming,
> 
> > I support adding make_bad_inode() in ocfs2_read_inode_block_full().
> > ocfs2_read_locked_inode() calls ocfs2_read_inode_block[_full] to read the inode
> > from disk. However, ocfs2_read_inode_block[_full] have many callers, and in
> > current code, only ocfs2_read_locked_inode() marks the inode as bad. All others
> > forget to set the bad_inode.
> > 
> > The 'forbid' write operations when read-only mode is worth another patch, and
> > I plan to create this patch. This patch adds a similar ext4_emergency_state()
> > function for ocfs2.
> 
> We're working on this as part of the Linux Kernel Mentorship Program, and we'd
> love to take on implementing the read-only check if it's not overly
> complicated. We're just beginners, but we thought it would be a great learning
> experience to work on this following the ext4 pattern you mentioned - if you
> haven't already started working on it by the time you see this reply.

I haven't started the patch job, you are welcome to take it.

- Heming
> 
> > Therefore, your original patch looks good to me. I will provide my Reivewed-by
> > for it.
> 
> Thank you so much for your review and the Reviewed-by tag
> 
> Cheers,
> 	Albin

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read
  2025-10-31 14:05             ` Heming Zhao
@ 2025-10-31 14:22               ` Albin Babu Varghese
  2025-11-04 22:31                 ` Ahmet Eray Karadag
  0 siblings, 1 reply; 12+ messages in thread
From: Albin Babu Varghese @ 2025-10-31 14:22 UTC (permalink / raw)
  To: Heming Zhao
  Cc: Ahmet Eray Karadag, mark, jlbec, joseph.qi, ocfs2-devel,
	linux-kernel, david.hunter.linux, skhan,
	syzbot+b93b65ee321c97861072

> > > I support adding make_bad_inode() in ocfs2_read_inode_block_full().
> > > ocfs2_read_locked_inode() calls ocfs2_read_inode_block[_full] to read the inode
> > > from disk. However, ocfs2_read_inode_block[_full] have many callers, and in
> > > current code, only ocfs2_read_locked_inode() marks the inode as bad. All others
> > > forget to set the bad_inode.
> > > 
> > > The 'forbid' write operations when read-only mode is worth another patch, and
> > > I plan to create this patch. This patch adds a similar ext4_emergency_state()
> > > function for ocfs2.
> > 
> > We're working on this as part of the Linux Kernel Mentorship Program, and we'd
> > love to take on implementing the read-only check if it's not overly
> > complicated. We're just beginners, but we thought it would be a great learning
> > experience to work on this following the ext4 pattern you mentioned - if you
> > haven't already started working on it by the time you see this reply.
> 
> I haven't started the patch job, you are welcome to take it.

Thank you! We'll work on it and send the patch for review.

Cheers,
	Albin

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read
  2025-10-31 14:22               ` Albin Babu Varghese
@ 2025-11-04 22:31                 ` Ahmet Eray Karadag
  2025-11-05  1:40                   ` Joseph Qi
  0 siblings, 1 reply; 12+ messages in thread
From: Ahmet Eray Karadag @ 2025-11-04 22:31 UTC (permalink / raw)
  To: ocfs2-devel
  Cc: Albin Babu Varghese, mark, jlbec, joseph.qi, linux-kernel,
	Heming Zhao, david.hunter.linux, skhan,
	syzbot+b93b65ee321c97861072

Hi folks,

I'm having some trouble getting xfstests-dev set up to test some ocfs2
changes. I can't seem to get the configuration working correctly to run
the ocfs2 test group.

Could anyone share some insights or pointers on the proper setup? Any
help would be much appreciated.

Cheers,
Ahmet Eray

Albin Babu Varghese <albinbabuvarghese20@gmail.com>, 31 Eki 2025 Cum,
17:22 tarihinde şunu yazdı:
>
> > > > I support adding make_bad_inode() in ocfs2_read_inode_block_full().
> > > > ocfs2_read_locked_inode() calls ocfs2_read_inode_block[_full] to read the inode
> > > > from disk. However, ocfs2_read_inode_block[_full] have many callers, and in
> > > > current code, only ocfs2_read_locked_inode() marks the inode as bad. All others
> > > > forget to set the bad_inode.
> > > >
> > > > The 'forbid' write operations when read-only mode is worth another patch, and
> > > > I plan to create this patch. This patch adds a similar ext4_emergency_state()
> > > > function for ocfs2.
> > >
> > > We're working on this as part of the Linux Kernel Mentorship Program, and we'd
> > > love to take on implementing the read-only check if it's not overly
> > > complicated. We're just beginners, but we thought it would be a great learning
> > > experience to work on this following the ext4 pattern you mentioned - if you
> > > haven't already started working on it by the time you see this reply.
> >
> > I haven't started the patch job, you are welcome to take it.
>
> Thank you! We'll work on it and send the patch for review.
>
> Cheers,
>         Albin

^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read
  2025-11-04 22:31                 ` Ahmet Eray Karadag
@ 2025-11-05  1:40                   ` Joseph Qi
  0 siblings, 0 replies; 12+ messages in thread
From: Joseph Qi @ 2025-11-05  1:40 UTC (permalink / raw)
  To: Ahmet Eray Karadag, ocfs2-devel
  Cc: Albin Babu Varghese, mark, jlbec, linux-kernel, Heming Zhao,
	david.hunter.linux, skhan, syzbot+b93b65ee321c97861072



On 2025/11/5 06:31, Ahmet Eray Karadag wrote:
> Hi folks,
> 
> I'm having some trouble getting xfstests-dev set up to test some ocfs2
> changes. I can't seem to get the configuration working correctly to run
> the ocfs2 test group.
> 
> Could anyone share some insights or pointers on the proper setup? Any
> help would be much appreciated.
> 
The basic steps is:
1. Configure a ocfs2 cluster, e.g:
$ cat /etc/ocfs2/cluster.conf
cluster:
        heartbeat_mode = local
        node_count = 1
        name = ocfs2cluster

node:
        number = 0
        cluster = ocfs2cluster
        ip_port = 7777
        ip_address = 127.0.0.1
        name = localhost

2. Online ocfs2 cluster:
$ /etc/init.d/o2cb.init online

3. Then run xfstests cases, e.g.:
$ ./check generic/001

Actually we prefer ocfs2tests to test ocfs2, which can be cloned at:
https://github.com/markfasheh/ocfs2-test

Thanks,
Joseph

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-11-05  1:40 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-29 22:57 [RFC RFT PATCH] ocfs2: Mark inode bad upon validation failure during read Ahmet Eray Karadag
2025-10-30  7:59 ` Heming Zhao
2025-10-30 22:07   ` Ahmet Eray Karadag
2025-10-31  2:30     ` Heming Zhao
2025-10-31  3:59       ` Albin Babu Varghese
2025-10-31  7:04         ` Heming Zhao
2025-10-31 13:54           ` Albin Babu Varghese
2025-10-31 14:05             ` Heming Zhao
2025-10-31 14:22               ` Albin Babu Varghese
2025-11-04 22:31                 ` Ahmet Eray Karadag
2025-11-05  1:40                   ` Joseph Qi
2025-10-31  7:10 ` Heming Zhao

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox