diff mbox series

[(REPOST)] hfs: don't use BUG() when we can continue

Message ID b6e39a3e-f7ce-4f7e-aa77-f6b146bd7c92@I-love.SAKURA.ne.jp (mailing list archive)
State New
Headers show
Series [(REPOST)] hfs: don't use BUG() when we can continue | expand

Commit Message

Tetsuo Handa Dec. 5, 2024, 1:45 p.m. UTC 
syzkaller can mount crafted filesystem images.
Don't crash the kernel when we can continue.

Reported-by: syzbot+97e301b4b82ae803d21b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=97e301b4b82ae803d21b
Tested-by: syzbot+97e301b4b82ae803d21b@syzkaller.appspotmail.com
Signed-off-by: Tetsuo Handa <penguin-kernel@I-love.SAKURA.ne.jp>
---
 fs/hfs/inode.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

Comments

Matthew Wilcox (Oracle) Dec. 5, 2024, 1:59 p.m. UTC | #1 
On Thu, Dec 05, 2024 at 10:45:11PM +0900, Tetsuo Handa wrote:
> syzkaller can mount crafted filesystem images.
> Don't crash the kernel when we can continue.

The one in hfs_test_inode() makes sense, but we should never get to
hfs_release_folio() or hfs_write_inode() with a bad inode, even with a
corrupted fs.  Right?

> +++ b/fs/hfs/inode.c
> @@ -81,7 +81,7 @@ static bool hfs_release_folio(struct folio *folio, gfp_t mask)
>  		tree = HFS_SB(sb)->cat_tree;
>  		break;
>  	default:
> -		BUG();
> +		pr_warn("unexpected inode %lu at %s()\n", inode->i_ino, __func__);
>  		return false;
>  	}
>  
> @@ -305,7 +305,7 @@ static int hfs_test_inode(struct inode *inode, void *data)
>  	case HFS_CDR_FIL:
>  		return inode->i_ino == be32_to_cpu(rec->file.FlNum);
>  	default:
> -		BUG();
> +		pr_warn("unexpected type %u at %s()\n", rec->type, __func__);
>  		return 1;
>  	}
>  }
> @@ -441,7 +441,7 @@ int hfs_write_inode(struct inode *inode, struct writeback_control *wbc)
>  			hfs_btree_write(HFS_SB(inode->i_sb)->cat_tree);
>  			return 0;
>  		default:
> -			BUG();
> +			pr_warn("unexpected inode %lu at %s()\n", inode->i_ino, __func__);
>  			return -EIO;
>  		}
>  	}
> -- 
> 2.47.0
> 
>
Tetsuo Handa Dec. 5, 2024, 2:14 p.m. UTC | #2 
On 2024/12/05 22:59, Matthew Wilcox wrote:
> On Thu, Dec 05, 2024 at 10:45:11PM +0900, Tetsuo Handa wrote:
>> syzkaller can mount crafted filesystem images.
>> Don't crash the kernel when we can continue.
> 
> The one in hfs_test_inode() makes sense, but we should never get to
> hfs_release_folio() or hfs_write_inode() with a bad inode, even with a
> corrupted fs.  Right?

Unfortunately, https://syzkaller.appspot.com/bug?extid=97e301b4b82ae803d21b
reports that we can reach hfs_write_inode() with a corrupted filesystem.
If we should never get to hfs_write_inode(), how do we want to handle
this problem? Is someone familiar with hfs enough to perform an in-kernel
fsck upon mounting?
diff mbox series

Patch

diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index a81ce7a740b9..794d710c3ae0 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -81,7 +81,7 @@  static bool hfs_release_folio(struct folio *folio, gfp_t mask)
 		tree = HFS_SB(sb)->cat_tree;
 		break;
 	default:
-		BUG();
+		pr_warn("unexpected inode %lu at %s()\n", inode->i_ino, __func__);
 		return false;
 	}
 
@@ -305,7 +305,7 @@  static int hfs_test_inode(struct inode *inode, void *data)
 	case HFS_CDR_FIL:
 		return inode->i_ino == be32_to_cpu(rec->file.FlNum);
 	default:
-		BUG();
+		pr_warn("unexpected type %u at %s()\n", rec->type, __func__);
 		return 1;
 	}
 }
@@ -441,7 +441,7 @@  int hfs_write_inode(struct inode *inode, struct writeback_control *wbc)
 			hfs_btree_write(HFS_SB(inode->i_sb)->cat_tree);
 			return 0;
 		default:
-			BUG();
+			pr_warn("unexpected inode %lu at %s()\n", inode->i_ino, __func__);
 			return -EIO;
 		}
 	}