Re: [PATCH] qnx6: validate di_filelevels in qnx6_iget() and fix mount error handling

[Damien Le Moal <dlemoal@kernel.org>]

On 9/20/26 07:25, Hui Peng wrote:
> Fix two issues in fs/qnx6/:
> 
> 1. In qnx6_iget(), reject raw_inode->di_filelevels > QNX6_PTR_MAX_LEVELS
>    so computing the maximum block count does not trigger shift-out-of-
>    bounds undefined behavior or out-of-bounds block pointer array walks.
> 2. In qnx6_fill_super() and qnx6_mmi_fill_super(), prevent MS_SILENT
>    from bypassing superblock magic validation and ensure sbi->sb_buf is
>    released exactly once on failure.

Please split this into 2 patches, one patch for each problem you fix.

> 
> Fixes: 5d026c724220 ("fs: initial qnx6fs addition")
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---
> diff --git a/fs/qnx6/inode.c b/fs/qnx6/inode.c
> index 6de49333acad..04214b829e10 100644
> --- a/fs/qnx6/inode.c
> +++ b/fs/qnx6/inode.c
> @@ -144,8 +144,10 @@ static unsigned qnx6_block_map(struct inode *inode, unsigned no)
>  		levelptr = (no >> bitdelta) & mask;
>  		ptr = ((__fs32 *)bh->b_data)[levelptr];
>  
> -		if (!qnx6_check_blockptr(ptr))
> +		if (!qnx6_check_blockptr(ptr)) {
> +			brelse(bh);
>  			return 0;
> +		}
>  
>  		block = qnx6_get_devblock(s, ptr);
>  		brelse(bh);
> @@ -397,12 +399,14 @@ static int qnx6_fill_super(struct super_block *s, struct fs_context *fc)
>  		sbi->sb_buf = bh1;
>  		sbi->sb = (struct qnx6_super_block *)bh1->b_data;
>  		brelse(bh2);
> +		bh2 = NULL;
>  		pr_info("superblock #1 active\n");
>  	} else {
>  		/* superblock #2 active */
>  		sbi->sb_buf = bh2;
>  		sbi->sb = (struct qnx6_super_block *)bh2->b_data;
>  		brelse(bh1);
> +		bh1 = NULL;
>  		pr_info("superblock #2 active\n");
>  	}
>  mmi_success:
> @@ -463,6 +467,8 @@ static int qnx6_fill_super(struct super_block *s, struct fs_context *fc)
>  out1:
>  	iput(sbi->inodes);
>  out:
> +	if (sbi->sb_buf && sbi->sb_buf != bh1 && sbi->sb_buf != bh2)
> +		brelse(sbi->sb_buf);
>  	brelse(bh1);
>  	brelse(bh2);
>  outnobh:
> @@ -560,6 +566,13 @@ struct inode *qnx6_iget(struct super_block *sb, unsigned ino)
>  	memcpy(&ei->di_block_ptr, &raw_inode->di_block_ptr,
>  				sizeof(raw_inode->di_block_ptr));
>  	ei->di_filelevels = raw_inode->di_filelevels;
> +	if (ei->di_filelevels > QNX6_PTR_MAX_LEVELS) {
> +		pr_err("invalid filelevels (%u) in inode %u\n",
> +		       ei->di_filelevels, ino);
> +		folio_release_kmap(folio, raw_inode);
> +		iget_failed(inode);
> +		return ERR_PTR(-EIO);
> +	}
>  
>  	if (S_ISREG(inode->i_mode)) {
>  		inode->i_fop = &generic_ro_fops;
> diff --git a/fs/qnx6/super_mmi.c b/fs/qnx6/super_mmi.c
> index b8afb6f388b2..28cb9322278e 100644
> --- a/fs/qnx6/super_mmi.c
> +++ b/fs/qnx6/super_mmi.c
> @@ -51,10 +51,9 @@ struct qnx6_super_block *qnx6_mmi_fill_super(struct super_block *s, int silent)
>  	sb1 = (struct qnx6_mmi_super_block *)bh1->b_data;
>  	sbi = QNX6_SB(s);
>  	if (fs32_to_cpu(sbi, sb1->sb_magic) != QNX6_SUPER_MAGIC) {
> -		if (!silent) {
> +		if (!silent)
>  			pr_err("wrong signature (magic) in superblock #1.\n");
> -			goto out;
> -		}
> +		goto out;
>  	}
>  
>  	/* checksum check - start at byte 8 and end at byte 512 */
> @@ -64,15 +63,16 @@ struct qnx6_super_block *qnx6_mmi_fill_super(struct super_block *s, int silent)
>  		goto out;
>  	}
>  
> -	/* calculate second superblock blocknumber */
> -	offset = fs32_to_cpu(sbi, sb1->sb_num_blocks) + QNX6_SUPERBLOCK_AREA /
> -					fs32_to_cpu(sbi, sb1->sb_blocksize);
> -
>  	/* set new blocksize */
>  	if (!sb_set_blocksize(s, fs32_to_cpu(sbi, sb1->sb_blocksize))) {
>  		pr_err("unable to set blocksize\n");
>  		goto out;
>  	}
> +
> +	/* calculate second superblock blocknumber */
> +	offset = fs32_to_cpu(sbi, sb1->sb_num_blocks) + QNX6_SUPERBLOCK_AREA /
> +					fs32_to_cpu(sbi, sb1->sb_blocksize);
> +
>  	/* blocksize invalidates bh - pull it back in */
>  	brelse(bh1);
>  	bh1 = sb_bread(s, 0);


-- 
Damien Le Moal
Western Digital Research