[<prev] [next>] [day] [month] [year] [list]
Message-ID: <20251125211329.2835801-1-jkoolstra@xs4all.nl>
Date: Tue, 25 Nov 2025 22:13:27 +0100
From: Jori Koolstra <jkoolstra@...all.nl>
To: linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org
Cc: slava@...eyko.com,
glaubitz@...sik.fu-berlin.de,
frank.li@...o.com,
skhan@...uxfoundation.org,
Jori Koolstra <jkoolstra@...all.nl>,
syzbot+17cc9bb6d8d69b4139f0@...kaller.appspotmail.com
Subject: [PATCH v2] hfs: replace BUG_ONs with error handling
In a06ec283e125 next_id, folder_count, and file_count in the super block
info were expanded to 64 bits, and BUG_ONs were added to detect
overflow. This triggered an error reported by syzbot: if the MDB is
corrupted, the BUG_ON is triggered. This patch replaces this mechanism
with proper error handling and resolves the syzbot reported bug.
Singed-off-by: Jori Koolstra <jkoolstra@...all.nl>
Reported-by: syzbot+17cc9bb6d8d69b4139f0@...kaller.appspotmail.com
Closes: https://syzbot.org/bug?extid=17cc9bb6d8d69b4139f0
Signed-off-by: Jori Koolstra <jkoolstra@...all.nl>
---
fs/hfs/dir.c | 12 ++++++------
fs/hfs/hfs.h | 3 +++
fs/hfs/hfs_fs.h | 2 +-
fs/hfs/inode.c | 40 ++++++++++++++++++++++++++++++++--------
fs/hfs/mdb.c | 15 ++++++++++++---
5 files changed, 54 insertions(+), 18 deletions(-)
diff --git a/fs/hfs/dir.c b/fs/hfs/dir.c
index 86a6b317b474..03881a91f869 100644
--- a/fs/hfs/dir.c
+++ b/fs/hfs/dir.c
@@ -196,8 +196,8 @@ static int hfs_create(struct mnt_idmap *idmap, struct inode *dir,
int res;
inode = hfs_new_inode(dir, &dentry->d_name, mode);
- if (!inode)
- return -ENOMEM;
+ if (IS_ERR(inode))
+ return PTR_ERR(inode);
res = hfs_cat_create(inode->i_ino, dir, &dentry->d_name, inode);
if (res) {
@@ -226,8 +226,8 @@ static struct dentry *hfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
int res;
inode = hfs_new_inode(dir, &dentry->d_name, S_IFDIR | mode);
- if (!inode)
- return ERR_PTR(-ENOMEM);
+ if (IS_ERR(inode))
+ return ERR_CAST(inode);
res = hfs_cat_create(inode->i_ino, dir, &dentry->d_name, inode);
if (res) {
@@ -264,9 +264,9 @@ static int hfs_remove(struct inode *dir, struct dentry *dentry)
return res;
clear_nlink(inode);
inode_set_ctime_current(inode);
- hfs_delete_inode(inode);
+ res = hfs_delete_inode(inode);
mark_inode_dirty(inode);
- return 0;
+ return res;
}
/*
diff --git a/fs/hfs/hfs.h b/fs/hfs/hfs.h
index 6f194d0768b6..4b4797ef4e50 100644
--- a/fs/hfs/hfs.h
+++ b/fs/hfs/hfs.h
@@ -287,3 +287,6 @@ struct hfs_readdir_data {
};
#endif
+
+
+#define EFSCORRUPTED EUCLEAN /* Filesystem is corrupted */
diff --git a/fs/hfs/hfs_fs.h b/fs/hfs/hfs_fs.h
index fff149af89da..21dfdde71b14 100644
--- a/fs/hfs/hfs_fs.h
+++ b/fs/hfs/hfs_fs.h
@@ -182,7 +182,7 @@ extern void hfs_inode_read_fork(struct inode *inode, struct hfs_extent *ext,
__be32 log_size, __be32 phys_size, u32 clump_size);
extern struct inode *hfs_iget(struct super_block *, struct hfs_cat_key *, hfs_cat_rec *);
extern void hfs_evict_inode(struct inode *);
-extern void hfs_delete_inode(struct inode *);
+extern int hfs_delete_inode(struct inode *);
/* attr.c */
extern const struct xattr_handler * const hfs_xattr_handlers[];
diff --git a/fs/hfs/inode.c b/fs/hfs/inode.c
index 9cd449913dc8..ce27d49c41e4 100644
--- a/fs/hfs/inode.c
+++ b/fs/hfs/inode.c
@@ -186,16 +186,22 @@ struct inode *hfs_new_inode(struct inode *dir, const struct qstr *name, umode_t
s64 next_id;
s64 file_count;
s64 folder_count;
+ int err = -ENOMEM;
if (!inode)
- return NULL;
+ goto out_err;
+
+ err = -EFSCORRUPTED;
mutex_init(&HFS_I(inode)->extents_lock);
INIT_LIST_HEAD(&HFS_I(inode)->open_dir_list);
spin_lock_init(&HFS_I(inode)->open_dir_lock);
hfs_cat_build_key(sb, (btree_key *)&HFS_I(inode)->cat_key, dir->i_ino, name);
next_id = atomic64_inc_return(&HFS_SB(sb)->next_id);
- BUG_ON(next_id > U32_MAX);
+ if (next_id > U32_MAX) {
+ pr_err("next CNID exceeds limit — filesystem corrupted. It is recommended to run fsck\n");
+ goto out_discard;
+ }
inode->i_ino = (u32)next_id;
inode->i_mode = mode;
inode->i_uid = current_fsuid();
@@ -209,7 +215,10 @@ struct inode *hfs_new_inode(struct inode *dir, const struct qstr *name, umode_t
if (S_ISDIR(mode)) {
inode->i_size = 2;
folder_count = atomic64_inc_return(&HFS_SB(sb)->folder_count);
- BUG_ON(folder_count > U32_MAX);
+ if (folder_count > U32_MAX) {
+ pr_err("folder count exceeds limit — filesystem corrupted. It is recommended to run fsck\n");
+ goto out_discard;
+ }
if (dir->i_ino == HFS_ROOT_CNID)
HFS_SB(sb)->root_dirs++;
inode->i_op = &hfs_dir_inode_operations;
@@ -219,7 +228,10 @@ struct inode *hfs_new_inode(struct inode *dir, const struct qstr *name, umode_t
} else if (S_ISREG(mode)) {
HFS_I(inode)->clump_blocks = HFS_SB(sb)->clumpablks;
file_count = atomic64_inc_return(&HFS_SB(sb)->file_count);
- BUG_ON(file_count > U32_MAX);
+ if (file_count > U32_MAX) {
+ pr_err("file count exceeds limit — filesystem corrupted. It is recommended to run fsck\n");
+ goto out_discard;
+ }
if (dir->i_ino == HFS_ROOT_CNID)
HFS_SB(sb)->root_files++;
inode->i_op = &hfs_file_inode_operations;
@@ -243,24 +255,35 @@ struct inode *hfs_new_inode(struct inode *dir, const struct qstr *name, umode_t
hfs_mark_mdb_dirty(sb);
return inode;
+
+ out_discard:
+ iput(inode);
+ out_err:
+ return ERR_PTR(err);
}
-void hfs_delete_inode(struct inode *inode)
+int hfs_delete_inode(struct inode *inode)
{
struct super_block *sb = inode->i_sb;
hfs_dbg("ino %lu\n", inode->i_ino);
if (S_ISDIR(inode->i_mode)) {
- BUG_ON(atomic64_read(&HFS_SB(sb)->folder_count) > U32_MAX);
+ if (atomic64_read(&HFS_SB(sb)->folder_count) > U32_MAX) {
+ pr_err("folder count exceeds limit — filesystem corrupted. It is recommended to run fsck\n");
+ return -EFSCORRUPTED;
+ }
atomic64_dec(&HFS_SB(sb)->folder_count);
if (HFS_I(inode)->cat_key.ParID == cpu_to_be32(HFS_ROOT_CNID))
HFS_SB(sb)->root_dirs--;
set_bit(HFS_FLG_MDB_DIRTY, &HFS_SB(sb)->flags);
hfs_mark_mdb_dirty(sb);
- return;
+ return 0;
}
- BUG_ON(atomic64_read(&HFS_SB(sb)->file_count) > U32_MAX);
+ if (atomic64_read(&HFS_SB(sb)->file_count) > U32_MAX) {
+ pr_err("file count exceeds limit — filesystem corrupted. It is recommended to run fsck\n");
+ return -EFSCORRUPTED;
+ }
atomic64_dec(&HFS_SB(sb)->file_count);
if (HFS_I(inode)->cat_key.ParID == cpu_to_be32(HFS_ROOT_CNID))
HFS_SB(sb)->root_files--;
@@ -272,6 +295,7 @@ void hfs_delete_inode(struct inode *inode)
}
set_bit(HFS_FLG_MDB_DIRTY, &HFS_SB(sb)->flags);
hfs_mark_mdb_dirty(sb);
+ return 0;
}
void hfs_inode_read_fork(struct inode *inode, struct hfs_extent *ext,
diff --git a/fs/hfs/mdb.c b/fs/hfs/mdb.c
index 53f3fae60217..45b690ab4ba5 100644
--- a/fs/hfs/mdb.c
+++ b/fs/hfs/mdb.c
@@ -273,15 +273,24 @@ void hfs_mdb_commit(struct super_block *sb)
/* These parameters may have been modified, so write them back */
mdb->drLsMod = hfs_mtime();
mdb->drFreeBks = cpu_to_be16(HFS_SB(sb)->free_ablocks);
- BUG_ON(atomic64_read(&HFS_SB(sb)->next_id) > U32_MAX);
+ if (atomic64_read(&HFS_SB(sb)->next_id) > U32_MAX) {
+ pr_err("next CNID exceeds limit — filesystem corrupted. It is recommended to run fsck\n");
+ return;
+ }
mdb->drNxtCNID =
cpu_to_be32((u32)atomic64_read(&HFS_SB(sb)->next_id));
mdb->drNmFls = cpu_to_be16(HFS_SB(sb)->root_files);
mdb->drNmRtDirs = cpu_to_be16(HFS_SB(sb)->root_dirs);
- BUG_ON(atomic64_read(&HFS_SB(sb)->file_count) > U32_MAX);
+ if (atomic64_read(&HFS_SB(sb)->file_count) > U32_MAX) {
+ pr_err("file count exceeds limit — filesystem corrupted. It is recommended to run fsck\n");
+ return;
+ }
mdb->drFilCnt =
cpu_to_be32((u32)atomic64_read(&HFS_SB(sb)->file_count));
- BUG_ON(atomic64_read(&HFS_SB(sb)->folder_count) > U32_MAX);
+ if (atomic64_read(&HFS_SB(sb)->folder_count) > U32_MAX) {
+ pr_err("folder count exceeds limit — filesystem corrupted. It is recommended to run fsck\n");
+ return;
+ }
mdb->drDirCnt =
cpu_to_be32((u32)atomic64_read(&HFS_SB(sb)->folder_count));
--
2.51.2
Powered by blists - more mailing lists