* [PATCH v6] hfsplus: validate b-tree fork extents at mount and inode read
@ 2026-09-18 8:51 Nguyen Ngoc Thang
2026-09-21 21:07 ` Viacheslav Dubeyko
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Nguyen Ngoc Thang @ 2026-09-18 8:51 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: John Paul Adrian Glaubitz, Yangtao Li, linux-fsdevel,
linux-kernel, syzbot+f8ce6c197125ab9d72ce, Nguyen Ngoc Thang
Validate fork extents during inode reading and mount time to catch
on-disk corruptions early, returning appropriate errors and marking
the tree as corrupted.
v6:
- Move fork validation logic into inode read fork function.
- Refactor extent validation helpers, use volume_blocks, count == 0,
and introduce HFSPLUS_EXTENT_LAST_IDX named constant.
- Return error from hfsplus_inode_read_fork() to allow hfsplus_iget()
to catch on-disk corruption and propagate error correctly.
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
fs/hfsplus/extents.c | 41 ++++++++++++++++++++++++++++
fs/hfsplus/inode.c | 65 ++++++++++++++++++++++++++------------------
2 files changed, 79 insertions(+), 27 deletions(-)
diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
index eb7c11524d18..eaf3bed7ede8 100644
--- a/fs/hfsplus/extents.c
+++ b/fs/hfsplus/extents.c
@@ -16,6 +16,47 @@
#include "hfsplus_fs.h"
#include "hfsplus_raw.h"
+/* Index of the last extent in the fork */
+#define HFSPLUS_EXTENT_LAST_IDX 7
+
+static inline bool is_extents_btree(struct inode *inode)
+{
+ return inode->i_ino == HFSPLUS_EXT_CNID;
+}
+
+static bool hfsplus_extent_valid(struct hfsplus_extent *ext, u32 volume_blocks)
+{
+ u32 start = be32_to_cpu(ext->start_block);
+ u32 count = be32_to_cpu(ext->block_count);
+
+ if (count == 0)
+ return start == 0;
+
+ return start + count <= volume_blocks;
+}
+
+/*
+ * Returns 0 if fork extents are consistent, -EUCLEAN if extents
+ * past the first are corrupt, or -EIO if the first extent is corrupt.
+ */
+int hfsplus_check_fork(struct super_block *sb, struct hfsplus_extent *ext, u32 volume_blocks)
+{
+ bool non_zero_seen = false;
+ int i;
+
+ for (i = 0; i <= HFSPLUS_EXTENT_LAST_IDX; i++, ext++) {
+ u32 count = be32_to_cpu(ext->block_count);
+
+ if (!hfsplus_extent_valid(ext, volume_blocks) || (non_zero_seen && count == 0))
+ return i ? -EUCLEAN : -EIO;
+
+ if (count > 0)
+ non_zero_seen = true;
+ }
+
+ return 0;
+}
+
/* Compare two extents keys, returns 0 on same, pos/neg for difference */
int hfsplus_ext_cmp_key(const hfsplus_btree_key *k1,
const hfsplus_btree_key *k2)
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index 2ce6de574fa6..aa201f4e80d5 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -559,34 +559,45 @@ void hfsplus_delete_inode(struct inode *inode)
hfsplus_mark_mdb_dirty(sb);
}
-void hfsplus_inode_read_fork(struct inode *inode, struct hfsplus_fork_raw *fork)
+int hfsplus_inode_read_fork(struct inode *inode, struct hfsplus_fork_raw *fork)
{
- struct super_block *sb = inode->i_sb;
- struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
- struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
- u32 count;
- int i;
-
- memcpy(&hip->first_extents, &fork->extents, sizeof(hfsplus_extent_rec));
- for (count = 0, i = 0; i < 8; i++)
- count += be32_to_cpu(fork->extents[i].block_count);
- hip->first_blocks = count;
- memset(hip->cached_extents, 0, sizeof(hfsplus_extent_rec));
- hip->cached_start = 0;
- hip->cached_blocks = 0;
-
- hip->alloc_blocks = be32_to_cpu(fork->total_blocks);
- hip->phys_size = inode->i_size = be64_to_cpu(fork->total_size);
- hip->fs_blocks =
- (inode->i_size + sb->s_blocksize - 1) >> sb->s_blocksize_bits;
- inode_set_bytes(inode, hip->fs_blocks << sb->s_blocksize_bits);
- hip->clump_blocks =
- be32_to_cpu(fork->clump_size) >> sbi->alloc_blksz_shift;
- if (!hip->clump_blocks) {
- hip->clump_blocks = HFSPLUS_IS_RSRC(inode) ?
- sbi->rsrc_clump_blocks :
- sbi->data_clump_blocks;
- }
+ struct super_block *sb = inode->i_sb;
+ struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
+ struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
+ u32 count;
+ int i, ret;
+
+ /* Validate fork extents to catch on-disk corruption early */
+ ret = hfsplus_check_fork(sb, fork->extents, sbi->total_blocks);
+ if (ret) {
+ pr_err("hfsplus: fork check failed for inode %lu (err=%d)\n", inode->i_ino, ret);
+ set_bit(HFSPLUS_I_CORRUPT_TREE, &hip->flags);
+ sb->s_flags |= SB_RDONLY;
+ return ret; /* Return error directly to the caller */
+ }
+
+ memcpy(&hip->first_extents, &fork->extents, sizeof(hfsplus_extent_rec));
+ for (count = 0, i = 0; i <= HFSPLUS_EXTENT_LAST_IDX; i++)
+ count += be32_to_cpu(fork->extents[i].block_count);
+ hip->first_blocks = count;
+ memset(hip->cached_extents, 0, sizeof(hfsplus_extent_rec));
+ hip->cached_start = 0;
+ hip->cached_blocks = 0;
+
+ hip->alloc_blocks = be32_to_cpu(fork->total_blocks);
+ hip->phys_size = inode->i_size = be64_to_cpu(fork->total_size);
+ hip->fs_blocks =
+ (inode->i_size + sb->s_blocksize - 1) >> sb->s_blocksize_bits;
+ inode_set_bytes(inode, hip->fs_blocks << sb->s_blocksize_bits);
+ hip->clump_blocks =
+ be32_to_cpu(fork->clump_size) >> sbi->alloc_blksz_shift;
+ if (!hip->clump_blocks) {
+ hip->clump_blocks = HFSPLUS_IS_RSRC(inode) ?
+ sbi->rsrc_clump_blocks :
+ sbi->data_clump_blocks;
+ }
+
+ return 0;
}
void hfsplus_inode_write_fork(struct inode *inode,
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v6] hfsplus: validate b-tree fork extents at mount and inode read
2026-09-18 8:51 [PATCH v6] hfsplus: validate b-tree fork extents at mount and inode read Nguyen Ngoc Thang
@ 2026-09-21 21:07 ` Viacheslav Dubeyko
2026-09-22 16:28 ` kernel test robot
2026-09-22 17:45 ` kernel test robot
2 siblings, 0 replies; 5+ messages in thread
From: Viacheslav Dubeyko @ 2026-09-21 21:07 UTC (permalink / raw)
To: Nguyen Ngoc Thang
Cc: John Paul Adrian Glaubitz, Yangtao Li, linux-fsdevel,
linux-kernel, syzbot+f8ce6c197125ab9d72ce
On Fri, 2026-09-18 at 15:51 +0700, Nguyen Ngoc Thang wrote:
> Validate fork extents during inode reading and mount time to catch
> on-disk corruptions early, returning appropriate errors and marking
> the tree as corrupted.
>
> v6:
> - Move fork validation logic into inode read fork function.
> - Refactor extent validation helpers, use volume_blocks, count == 0,
> and introduce HFSPLUS_EXTENT_LAST_IDX named constant.
> - Return error from hfsplus_inode_read_fork() to allow
> hfsplus_iget()
> to catch on-disk corruption and propagate error correctly.
This patch looks like the small portion of the whole fix. As far as I
can see, this patch has lost necessary portion of fixes that we had
before.
>
> Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
> ---
> fs/hfsplus/extents.c | 41 ++++++++++++++++++++++++++++
> fs/hfsplus/inode.c | 65 ++++++++++++++++++++++++++----------------
> --
> 2 files changed, 79 insertions(+), 27 deletions(-)
>
> diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
> index eb7c11524d18..eaf3bed7ede8 100644
> --- a/fs/hfsplus/extents.c
> +++ b/fs/hfsplus/extents.c
> @@ -16,6 +16,47 @@
> #include "hfsplus_fs.h"
> #include "hfsplus_raw.h"
>
> +/* Index of the last extent in the fork */
> +#define HFSPLUS_EXTENT_LAST_IDX 7
I think we need to place this declaration into hfsplus_fs.h.
> +
> +static inline bool is_extents_btree(struct inode *inode)
> +{
> + return inode->i_ino == HFSPLUS_EXT_CNID;
> +}
> +
> +static bool hfsplus_extent_valid(struct hfsplus_extent *ext, u32
> volume_blocks)
> +{
> + u32 start = be32_to_cpu(ext->start_block);
> + u32 count = be32_to_cpu(ext->block_count);
> +
> + if (count == 0)
> + return start == 0;
> +
> + return start + count <= volume_blocks;
> +}
> +
> +/*
> + * Returns 0 if fork extents are consistent, -EUCLEAN if extents
> + * past the first are corrupt, or -EIO if the first extent is
> corrupt.
> + */
> +int hfsplus_check_fork(struct super_block *sb, struct hfsplus_extent
> *ext, u32 volume_blocks)
> +{
> + bool non_zero_seen = false;
> + int i;
> +
> + for (i = 0; i <= HFSPLUS_EXTENT_LAST_IDX; i++, ext++) {
> + u32 count = be32_to_cpu(ext->block_count);
> +
> + if (!hfsplus_extent_valid(ext, volume_blocks) ||
> (non_zero_seen && count == 0))
> + return i ? -EUCLEAN : -EIO;
> +
> + if (count > 0)
> + non_zero_seen = true;
> + }
> +
> + return 0;
> +}
This logic doesn't look like the fork check.
struct hfsplus_fork_raw {
__be64 total_size;
__be32 clump_size;
__be32 total_blocks;
hfsplus_extent_rec extents;
} __packed;
We need to be sure that total_size, total_blocks are consistent with
the extents state. Also, we can check the clump_size that it is
reasonable one.
> +
> /* Compare two extents keys, returns 0 on same, pos/neg for
> difference */
> int hfsplus_ext_cmp_key(const hfsplus_btree_key *k1,
> const hfsplus_btree_key *k2)
> diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
> index 2ce6de574fa6..aa201f4e80d5 100644
> --- a/fs/hfsplus/inode.c
> +++ b/fs/hfsplus/inode.c
> @@ -559,34 +559,45 @@ void hfsplus_delete_inode(struct inode *inode)
> hfsplus_mark_mdb_dirty(sb);
> }
>
> -void hfsplus_inode_read_fork(struct inode *inode, struct
> hfsplus_fork_raw *fork)
> +int hfsplus_inode_read_fork(struct inode *inode, struct
> hfsplus_fork_raw *fork)
This patch hasn't any logic of checking the error code of
hfsplus_inode_read_fork().
> {
> - struct super_block *sb = inode->i_sb;
> - struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
> - struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
> - u32 count;
> - int i;
> -
> - memcpy(&hip->first_extents, &fork->extents,
> sizeof(hfsplus_extent_rec));
> - for (count = 0, i = 0; i < 8; i++)
> - count += be32_to_cpu(fork->extents[i].block_count);
> - hip->first_blocks = count;
> - memset(hip->cached_extents, 0, sizeof(hfsplus_extent_rec));
> - hip->cached_start = 0;
> - hip->cached_blocks = 0;
> -
> - hip->alloc_blocks = be32_to_cpu(fork->total_blocks);
> - hip->phys_size = inode->i_size = be64_to_cpu(fork-
> >total_size);
> - hip->fs_blocks =
> - (inode->i_size + sb->s_blocksize - 1) >> sb-
> >s_blocksize_bits;
> - inode_set_bytes(inode, hip->fs_blocks << sb-
> >s_blocksize_bits);
> - hip->clump_blocks =
> - be32_to_cpu(fork->clump_size) >> sbi-
> >alloc_blksz_shift;
> - if (!hip->clump_blocks) {
> - hip->clump_blocks = HFSPLUS_IS_RSRC(inode) ?
> - sbi->rsrc_clump_blocks :
> - sbi->data_clump_blocks;
> - }
> + struct super_block *sb = inode->i_sb;
> + struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
> + struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
> + u32 count;
> + int i, ret;
> +
> + /* Validate fork extents to catch on-disk corruption early */
> + ret = hfsplus_check_fork(sb, fork->extents, sbi->total_blocks);
We need to check the whole fork but not only extents.
> + if (ret) {
> + pr_err("hfsplus: fork check failed for inode %lu
> (err=%d)\n", inode->i_ino, ret);
> + set_bit(HFSPLUS_I_CORRUPT_TREE, &hip->flags);
> + sb->s_flags |= SB_RDONLY;
NO, we cannot set SB_RDONLY in this method. It can be done in
hfsplus_fill_super() or hfsplus_reconfigure().
> + return ret; /* Return error directly to the caller */
> + }
> +
> + memcpy(&hip->first_extents, &fork->extents,
> sizeof(hfsplus_extent_rec));
> + for (count = 0, i = 0; i <= HFSPLUS_EXTENT_LAST_IDX; i++)
> + count += be32_to_cpu(fork->extents[i].block_count);
Looks like a mess and improper formatting of the code. What's happen
here?
Thanks,
Slava.
> + hip->first_blocks = count;
> + memset(hip->cached_extents, 0, sizeof(hfsplus_extent_rec));
> + hip->cached_start = 0;
> + hip->cached_blocks = 0;
> +
> + hip->alloc_blocks = be32_to_cpu(fork->total_blocks);
> + hip->phys_size = inode->i_size = be64_to_cpu(fork->total_size);
> + hip->fs_blocks =
> + (inode->i_size + sb->s_blocksize - 1) >> sb-
> >s_blocksize_bits;
> + inode_set_bytes(inode, hip->fs_blocks << sb->s_blocksize_bits);
> + hip->clump_blocks =
> + be32_to_cpu(fork->clump_size) >> sbi->alloc_blksz_shift;
> + if (!hip->clump_blocks) {
> + hip->clump_blocks = HFSPLUS_IS_RSRC(inode) ?
> + sbi->rsrc_clump_blocks :
> + sbi->data_clump_blocks;
> + }
> +
> + return 0;
> }
>
> void hfsplus_inode_write_fork(struct inode *inode,
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v6] hfsplus: validate b-tree fork extents at mount and inode read
2026-09-18 8:51 [PATCH v6] hfsplus: validate b-tree fork extents at mount and inode read Nguyen Ngoc Thang
2026-09-21 21:07 ` Viacheslav Dubeyko
@ 2026-09-22 16:28 ` kernel test robot
2026-09-22 17:45 ` kernel test robot
2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2026-09-22 16:28 UTC (permalink / raw)
To: Nguyen Ngoc Thang, Viacheslav Dubeyko
Cc: oe-kbuild-all, John Paul Adrian Glaubitz, Yangtao Li,
linux-fsdevel, linux-kernel, syzbot+f8ce6c197125ab9d72ce,
Nguyen Ngoc Thang
Hi Nguyen,
kernel test robot noticed the following build errors:
[auto build test ERROR on brauner-vfs/vfs.all]
[also build test ERROR on linus/master v7.3-rc4 next-20260921]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Nguyen-Ngoc-Thang/hfsplus-validate-b-tree-fork-extents-at-mount-and-inode-read/20260918-155156
base: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link: https://lore.kernel.org/r/20260918085156.920842-1-ngocthang2710.1999%40gmail.com
patch subject: [PATCH v6] hfsplus: validate b-tree fork extents at mount and inode read
config: powerpc64-randconfig-1000-20260922 (https://download.01.org/0day-ci/archive/20260923/202609230024.oUY5bARR-lkp@intel.com/config)
compiler: powerpc64-linux-gcc (GCC) 8.5.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260923/202609230024.oUY5bARR-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202609230024.oUY5bARR-lkp@intel.com/
All error/warnings (new ones prefixed by >>):
>> fs/hfsplus/inode.c:562:5: error: conflicting types for 'hfsplus_inode_read_fork'
int hfsplus_inode_read_fork(struct inode *inode, struct hfsplus_fork_raw *fork)
^~~~~~~~~~~~~~~~~~~~~~~
In file included from fs/hfsplus/inode.c:22:
fs/hfsplus/hfsplus_fs.h:456:6: note: previous declaration of 'hfsplus_inode_read_fork' was here
void hfsplus_inode_read_fork(struct inode *inode,
^~~~~~~~~~~~~~~~~~~~~~~
fs/hfsplus/inode.c: In function 'hfsplus_inode_read_fork':
>> fs/hfsplus/inode.c:571:11: error: implicit declaration of function 'hfsplus_check_fork'; did you mean 'hfsplus_free_fork'? [-Werror=implicit-function-declaration]
ret = hfsplus_check_fork(sb, fork->extents, sbi->total_blocks);
^~~~~~~~~~~~~~~~~~
hfsplus_free_fork
In file included from include/asm-generic/bug.h:31,
from arch/powerpc/include/asm/bug.h:116,
from include/linux/bug.h:5,
from include/linux/instrumented.h:10,
from include/asm-generic/bitops/instrumented-atomic.h:14,
from arch/powerpc/include/asm/bitops.h:321,
from include/linux/bitops.h:67,
from include/linux/bitmap.h:8,
from include/linux/nodemask.h:91,
from include/linux/list_lru.h:12,
from include/linux/fs/super_types.h:7,
from include/linux/fs/super.h:5,
from include/linux/fs.h:5,
from include/linux/highmem.h:5,
from include/linux/bvec.h:10,
from include/linux/blk_types.h:10,
from include/linux/blkdev.h:9,
from fs/hfsplus/inode.c:12:
>> include/linux/kern_levels.h:5:18: warning: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'u64' {aka 'long long unsigned int'} [-Wformat=]
#define KERN_SOH "\001" /* ASCII Start Of Header */
^~~~~~
include/linux/printk.h:483:11: note: in definition of macro 'printk_index_wrap'
_p_func(_fmt, ##__VA_ARGS__); \
^~~~
include/linux/printk.h:554:2: note: in expansion of macro 'printk'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~
include/linux/kern_levels.h:11:18: note: in expansion of macro 'KERN_SOH'
#define KERN_ERR KERN_SOH "3" /* error conditions */
^~~~~~~~
include/linux/printk.h:554:9: note: in expansion of macro 'KERN_ERR'
printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
^~~~~~~~
fs/hfsplus/inode.c:573:2: note: in expansion of macro 'pr_err'
pr_err("hfsplus: fork check failed for inode %lu (err=%d)\n", inode->i_ino, ret);
^~~~~~
>> fs/hfsplus/inode.c:574:10: error: 'HFSPLUS_I_CORRUPT_TREE' undeclared (first use in this function); did you mean 'HFSPLUS_I_CAT_DIRTY'?
set_bit(HFSPLUS_I_CORRUPT_TREE, &hip->flags);
^~~~~~~~~~~~~~~~~~~~~~
HFSPLUS_I_CAT_DIRTY
fs/hfsplus/inode.c:574:10: note: each undeclared identifier is reported only once for each function it appears in
>> fs/hfsplus/inode.c:580:33: error: 'HFSPLUS_EXTENT_LAST_IDX' undeclared (first use in this function); did you mean 'HFSPLUS_EXT_CNID'?
for (count = 0, i = 0; i <= HFSPLUS_EXTENT_LAST_IDX; i++)
^~~~~~~~~~~~~~~~~~~~~~~
HFSPLUS_EXT_CNID
cc1: some warnings being treated as errors
--
>> fs/hfsplus/extents.c:42:5: warning: no previous prototype for 'hfsplus_check_fork' [-Wmissing-prototypes]
int hfsplus_check_fork(struct super_block *sb, struct hfsplus_extent *ext, u32 volume_blocks)
^~~~~~~~~~~~~~~~~~
vim +/hfsplus_inode_read_fork +562 fs/hfsplus/inode.c
561
> 562 int hfsplus_inode_read_fork(struct inode *inode, struct hfsplus_fork_raw *fork)
563 {
564 struct super_block *sb = inode->i_sb;
565 struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
566 struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
567 u32 count;
568 int i, ret;
569
570 /* Validate fork extents to catch on-disk corruption early */
> 571 ret = hfsplus_check_fork(sb, fork->extents, sbi->total_blocks);
572 if (ret) {
573 pr_err("hfsplus: fork check failed for inode %lu (err=%d)\n", inode->i_ino, ret);
> 574 set_bit(HFSPLUS_I_CORRUPT_TREE, &hip->flags);
575 sb->s_flags |= SB_RDONLY;
576 return ret; /* Return error directly to the caller */
577 }
578
579 memcpy(&hip->first_extents, &fork->extents, sizeof(hfsplus_extent_rec));
> 580 for (count = 0, i = 0; i <= HFSPLUS_EXTENT_LAST_IDX; i++)
581 count += be32_to_cpu(fork->extents[i].block_count);
582 hip->first_blocks = count;
583 memset(hip->cached_extents, 0, sizeof(hfsplus_extent_rec));
584 hip->cached_start = 0;
585 hip->cached_blocks = 0;
586
587 hip->alloc_blocks = be32_to_cpu(fork->total_blocks);
588 hip->phys_size = inode->i_size = be64_to_cpu(fork->total_size);
589 hip->fs_blocks =
590 (inode->i_size + sb->s_blocksize - 1) >> sb->s_blocksize_bits;
591 inode_set_bytes(inode, hip->fs_blocks << sb->s_blocksize_bits);
592 hip->clump_blocks =
593 be32_to_cpu(fork->clump_size) >> sbi->alloc_blksz_shift;
594 if (!hip->clump_blocks) {
595 hip->clump_blocks = HFSPLUS_IS_RSRC(inode) ?
596 sbi->rsrc_clump_blocks :
597 sbi->data_clump_blocks;
598 }
599
600 return 0;
601 }
602
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 5+ messages in thread* Re: [PATCH v6] hfsplus: validate b-tree fork extents at mount and inode read
2026-09-18 8:51 [PATCH v6] hfsplus: validate b-tree fork extents at mount and inode read Nguyen Ngoc Thang
2026-09-21 21:07 ` Viacheslav Dubeyko
2026-09-22 16:28 ` kernel test robot
@ 2026-09-22 17:45 ` kernel test robot
2 siblings, 0 replies; 5+ messages in thread
From: kernel test robot @ 2026-09-22 17:45 UTC (permalink / raw)
To: Nguyen Ngoc Thang, Viacheslav Dubeyko
Cc: oe-kbuild-all, John Paul Adrian Glaubitz, Yangtao Li,
linux-fsdevel, linux-kernel, syzbot+f8ce6c197125ab9d72ce,
Nguyen Ngoc Thang
Hi Nguyen,
kernel test robot noticed the following build errors:
[auto build test ERROR on brauner-vfs/vfs.all]
[also build test ERROR on linus/master v7.3-rc4 next-20260921]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]
url: https://github.com/intel-lab-lkp/linux/commits/Nguyen-Ngoc-Thang/hfsplus-validate-b-tree-fork-extents-at-mount-and-inode-read/20260918-155156
base: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.all
patch link: https://lore.kernel.org/r/20260918085156.920842-1-ngocthang2710.1999%40gmail.com
patch subject: [PATCH v6] hfsplus: validate b-tree fork extents at mount and inode read
config: sparc64-randconfig-1000-20260922 (https://download.01.org/0day-ci/archive/20260923/202609230123.7x3OfA7z-lkp@intel.com/config)
compiler: sparc64-linux-gcc (GCC) 15.2.0
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260923/202609230123.7x3OfA7z-lkp@intel.com/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <lkp@intel.com>
| Closes: https://lore.kernel.org/oe-kbuild-all/202609230123.7x3OfA7z-lkp@intel.com/
All errors (new ones prefixed by >>):
>> fs/hfsplus/inode.c:562:5: error: conflicting types for 'hfsplus_inode_read_fork'; have 'int(struct inode *, struct hfsplus_fork_raw *)'
562 | int hfsplus_inode_read_fork(struct inode *inode, struct hfsplus_fork_raw *fork)
| ^~~~~~~~~~~~~~~~~~~~~~~
In file included from fs/hfsplus/inode.c:22:
fs/hfsplus/hfsplus_fs.h:456:6: note: previous declaration of 'hfsplus_inode_read_fork' with type 'void(struct inode *, struct hfsplus_fork_raw *)'
456 | void hfsplus_inode_read_fork(struct inode *inode,
| ^~~~~~~~~~~~~~~~~~~~~~~
fs/hfsplus/inode.c: In function 'hfsplus_inode_read_fork':
fs/hfsplus/inode.c:571:11: error: implicit declaration of function 'hfsplus_check_fork'; did you mean 'hfsplus_free_fork'? [-Wimplicit-function-declaration]
571 | ret = hfsplus_check_fork(sb, fork->extents, sbi->total_blocks);
| ^~~~~~~~~~~~~~~~~~
| hfsplus_free_fork
In file included from include/asm-generic/bug.h:31,
from arch/sparc/include/asm/bug.h:25,
from include/linux/bug.h:5,
from include/linux/instrumented.h:10,
from include/linux/atomic/atomic-instrumented.h:17,
from include/linux/atomic.h:82,
from include/asm-generic/bitops/lock.h:5,
from arch/sparc/include/asm/bitops_64.h:52,
from arch/sparc/include/asm/bitops.h:5,
from include/linux/bitops.h:67,
from include/linux/bitmap.h:8,
from include/linux/nodemask.h:91,
from include/linux/list_lru.h:12,
from include/linux/fs/super_types.h:7,
from include/linux/fs/super.h:5,
from include/linux/fs.h:5,
from include/linux/highmem.h:5,
from include/linux/bvec.h:10,
from include/linux/blk_types.h:10,
from include/linux/blkdev.h:9,
from fs/hfsplus/inode.c:12:
include/linux/kern_levels.h:5:25: warning: format '%lu' expects argument of type 'long unsigned int', but argument 2 has type 'u64' {aka 'long long unsigned int'} [-Wformat=]
5 | #define KERN_SOH "\001" /* ASCII Start Of Header */
| ^~~~~~
include/linux/printk.h:483:25: note: in definition of macro 'printk_index_wrap'
483 | _p_func(_fmt, ##__VA_ARGS__); \
| ^~~~
include/linux/printk.h:554:9: note: in expansion of macro 'printk'
554 | printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~
include/linux/kern_levels.h:11:25: note: in expansion of macro 'KERN_SOH'
11 | #define KERN_ERR KERN_SOH "3" /* error conditions */
| ^~~~~~~~
include/linux/printk.h:554:16: note: in expansion of macro 'KERN_ERR'
554 | printk(KERN_ERR pr_fmt(fmt), ##__VA_ARGS__)
| ^~~~~~~~
fs/hfsplus/inode.c:573:9: note: in expansion of macro 'pr_err'
573 | pr_err("hfsplus: fork check failed for inode %lu (err=%d)\n", inode->i_ino, ret);
| ^~~~~~
fs/hfsplus/inode.c:574:17: error: 'HFSPLUS_I_CORRUPT_TREE' undeclared (first use in this function)
574 | set_bit(HFSPLUS_I_CORRUPT_TREE, &hip->flags);
| ^~~~~~~~~~~~~~~~~~~~~~
fs/hfsplus/inode.c:574:17: note: each undeclared identifier is reported only once for each function it appears in
fs/hfsplus/inode.c:580:33: error: 'HFSPLUS_EXTENT_LAST_IDX' undeclared (first use in this function)
580 | for (count = 0, i = 0; i <= HFSPLUS_EXTENT_LAST_IDX; i++)
| ^~~~~~~~~~~~~~~~~~~~~~~
vim +562 fs/hfsplus/inode.c
561
> 562 int hfsplus_inode_read_fork(struct inode *inode, struct hfsplus_fork_raw *fork)
563 {
564 struct super_block *sb = inode->i_sb;
565 struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
566 struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
567 u32 count;
568 int i, ret;
569
570 /* Validate fork extents to catch on-disk corruption early */
571 ret = hfsplus_check_fork(sb, fork->extents, sbi->total_blocks);
572 if (ret) {
573 pr_err("hfsplus: fork check failed for inode %lu (err=%d)\n", inode->i_ino, ret);
574 set_bit(HFSPLUS_I_CORRUPT_TREE, &hip->flags);
575 sb->s_flags |= SB_RDONLY;
576 return ret; /* Return error directly to the caller */
577 }
578
579 memcpy(&hip->first_extents, &fork->extents, sizeof(hfsplus_extent_rec));
580 for (count = 0, i = 0; i <= HFSPLUS_EXTENT_LAST_IDX; i++)
581 count += be32_to_cpu(fork->extents[i].block_count);
582 hip->first_blocks = count;
583 memset(hip->cached_extents, 0, sizeof(hfsplus_extent_rec));
584 hip->cached_start = 0;
585 hip->cached_blocks = 0;
586
587 hip->alloc_blocks = be32_to_cpu(fork->total_blocks);
588 hip->phys_size = inode->i_size = be64_to_cpu(fork->total_size);
589 hip->fs_blocks =
590 (inode->i_size + sb->s_blocksize - 1) >> sb->s_blocksize_bits;
591 inode_set_bytes(inode, hip->fs_blocks << sb->s_blocksize_bits);
592 hip->clump_blocks =
593 be32_to_cpu(fork->clump_size) >> sbi->alloc_blksz_shift;
594 if (!hip->clump_blocks) {
595 hip->clump_blocks = HFSPLUS_IS_RSRC(inode) ?
596 sbi->rsrc_clump_blocks :
597 sbi->data_clump_blocks;
598 }
599
600 return 0;
601 }
602
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v6] hfsplus: validate b-tree fork extents at mount and inode read
@ 2026-09-18 8:39 Nguyen Ngoc Thang
0 siblings, 0 replies; 5+ messages in thread
From: Nguyen Ngoc Thang @ 2026-09-18 8:39 UTC (permalink / raw)
To: Viacheslav Dubeyko
Cc: John Paul Adrian Glaubitz, Yangtao Li, linux-fsdevel,
linux-kernel, syzbot+f8ce6c197125ab9d72ce, Nguyen Ngoc Thang
Validate fork extents during inode reading and mount time to catch
on-disk corruptions early, returning appropriate errors and marking
the tree as corrupted.
v6:
- Move fork validation logic into inode read fork function.
- Refactor extent validation helpers, use volume_blocks, count == 0,
and introduce HFSPLUS_EXTENT_LAST_IDX named constant.
- Return error from hfsplus_inode_read_fork() to allow hfsplus_iget()
to catch on-disk corruption and propagate error correctly.
Signed-off-by: Nguyen Ngoc Thang <ngocthang2710.1999@gmail.com>
---
fs/hfsplus/extents.c | 41 ++++++++++++++++++++++++++++
fs/hfsplus/inode.c | 65 ++++++++++++++++++++++++++------------------
2 files changed, 79 insertions(+), 27 deletions(-)
diff --git a/fs/hfsplus/extents.c b/fs/hfsplus/extents.c
index eb7c11524d18..737cf113f215 100644
--- a/fs/hfsplus/extents.c
+++ b/fs/hfsplus/extents.c
@@ -16,6 +16,47 @@
#include "hfsplus_fs.h"
#include "hfsplus_raw.h"
+/* Index of the last extent in the fork */
+#define HFSPLUS_EXTENT_LAST_IDX 7
+
+static inline bool is_extents_btree(struct inode *inode)
+{
+ return inode->i_ino == HFSPLUS_EXT_CNID;
+}
+
+static bool hfsplus_extent_valid(struct hfsplus_extent *ext, u32 volume_blocks)
+{
+ u32 start = be32_to_cpu(ext->start_block);
+ u32 count = be32_to_cpu(ext->block_count);
+
+ if (count == 0)
+ return start == 0;
+
+ return start + count <= volume_blocks;
+}
+
+/*
+ * Returns 0 if fork extents are consistent, -EUCLEAN if extents
+ * past the first are corrupt, or -EIO if the first extent is corrupt.
+ */
+int hfsplus_check_fork(struct super_block *sb, struct hfsplus_extent *ext, u32 volume_blocks)
+{
+ bool non_zero_seen = false;
+ int i;
+
+ for (i = 0; i <= HFSPLUS_EXTENT_LAST_IDX; i++, ext++) {
+ u32 count = be32_to_cpu(ext->block_count);
+
+ if (!hfsplus_extent_valid(ext, volume_blocks) || (non_zero_seen && count == 0))
+ return i ? -EUCLEAN : -EIO;
+
+ if (count > 0)
+ non_zero_seen = true;
+ }
+
+ return 0;
+}
+
/* Compare two extents keys, returns 0 on same, pos/neg for difference */
int hfsplus_ext_cmp_key(const hfsplus_btree_key *k1,
const hfsplus_btree_key *k2)
diff --git a/fs/hfsplus/inode.c b/fs/hfsplus/inode.c
index 2ce6de574fa6..57fedd0e80fb 100644
--- a/fs/hfsplus/inode.c
+++ b/fs/hfsplus/inode.c
@@ -559,34 +559,45 @@ void hfsplus_delete_inode(struct inode *inode)
hfsplus_mark_mdb_dirty(sb);
}
-void hfsplus_inode_read_fork(struct inode *inode, struct hfsplus_fork_raw *fork)
+int hfsplus_inode_read_fork(struct inode *inode, struct hfsplus_fork_raw *fork)
{
- struct super_block *sb = inode->i_sb;
- struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
- struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
- u32 count;
- int i;
-
- memcpy(&hip->first_extents, &fork->extents, sizeof(hfsplus_extent_rec));
- for (count = 0, i = 0; i < 8; i++)
- count += be32_to_cpu(fork->extents[i].block_count);
- hip->first_blocks = count;
- memset(hip->cached_extents, 0, sizeof(hfsplus_extent_rec));
- hip->cached_start = 0;
- hip->cached_blocks = 0;
-
- hip->alloc_blocks = be32_to_cpu(fork->total_blocks);
- hip->phys_size = inode->i_size = be64_to_cpu(fork->total_size);
- hip->fs_blocks =
- (inode->i_size + sb->s_blocksize - 1) >> sb->s_blocksize_bits;
- inode_set_bytes(inode, hip->fs_blocks << sb->s_blocksize_bits);
- hip->clump_blocks =
- be32_to_cpu(fork->clump_size) >> sbi->alloc_blksz_shift;
- if (!hip->clump_blocks) {
- hip->clump_blocks = HFSPLUS_IS_RSRC(inode) ?
- sbi->rsrc_clump_blocks :
- sbi->data_clump_blocks;
- }
+ struct super_block *sb = inode->i_sb;
+ struct hfsplus_sb_info *sbi = HFSPLUS_SB(sb);
+ struct hfsplus_inode_info *hip = HFSPLUS_I(inode);
+ u32 count;
+ int i, ret;
+
+ /* Validate fork extents to catch on-disk corruption early */
+ ret = hfsplus_check_fork(sb, fork->extents, sbi->total_blocks);
+ if (ret) {
+ pr_err("hfsplus: fork check failed for inode %lu (err=%d)\n", inode->i_ino, ret);
+ set_bit(HFSPLUS_I_CORRUPT_TREE, &hip->flags);
+ sb->s_flags |= SB_RDONLY;
+ return ret; /* Return error directly to the caller */
+ }
+
+ memcpy(&hip->first_extents, &fork->extents, sizeof(hfsplus_extent_rec));
+ for (count = 0, i = 0; i <= HFSPLUS_EXTENT_LAST_IDX; i++)
+ count += be32_to_cpu(fork->extents[i].block_count);
+ hip->first_blocks = count;
+ memset(hip->cached_extents, 0, sizeof(hfsplus_extent_rec));
+ hip->cached_start = 0;
+ hip->cached_blocks = 0;
+
+ hip->alloc_blocks = be32_to_cpu(fork->total_blocks);
+ hip->phys_size = inode->i_size = be64_to_cpu(fork->total_size);
+ hip->fs_blocks =
+ (inode->i_size + sb->s_blocksize - 1) >> sb->s_blocksize_bits;
+ inode_set_bytes(inode, hip->fs_blocks << sb->s_blocksize_bits);
+ hip->clump_blocks =
+ be32_to_cpu(fork->clump_size) >> sbi->alloc_blksz_shift;
+ if (!hip->clump_blocks) {
+ hip->clump_blocks = HFSPLUS_IS_RSRC(inode) ?
+ sbi->rsrc_clump_blocks :
+ sbi->data_clump_blocks;
+ }
+
+ return 0;
}
void hfsplus_inode_write_fork(struct inode *inode,
--
2.43.0
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-09-22 17:45 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-18 8:51 [PATCH v6] hfsplus: validate b-tree fork extents at mount and inode read Nguyen Ngoc Thang
2026-09-21 21:07 ` Viacheslav Dubeyko
2026-09-22 16:28 ` kernel test robot
2026-09-22 17:45 ` kernel test robot
-- strict thread matches above, loose matches on Subject: below --
2026-09-18 8:39 Nguyen Ngoc Thang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®