mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [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
  2026-09-22 16:28 ` kernel test robot
  0 siblings, 2 replies; 4+ 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] 4+ 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
  1 sibling, 0 replies; 4+ 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] 4+ 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
  1 sibling, 0 replies; 4+ 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] 4+ 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; 4+ 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] 4+ messages in thread

end of thread, other threads:[~2026-09-22 16:29 UTC | newest]

Thread overview: 4+ 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
  -- 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®