[PATCH 0/2] ufs: harden the mount path against malformed images

[PATCH 0/2] ufs: harden the mount path against malformed images

[Ali Ahmet Memis]

Two robustness fixes for the ufs mount path, both reachable by mounting
a crafted UFS image.

Patch 1 reorders ufs_fill_super() so the root dentry is created after the
cylinder group metadata is loaded. Today a failure while reading the
cylinder groups drops UFS_SB(sb) but leaves s_root installed, so the
generic teardown oopses on a NULL pointer in ufs_sync_fs() and the
put_super operation.

Patch 2 validates the cylinder group index and the rotor positions in
ufs_read_cylinder() before they are cached. Unchecked, c_cgx indexes the
cylinder summary array (a 32 bit write outside s_csp), and the rotors
become bitmap scan offsets that can wrap an unsigned length and walk past
the cylinder group buffers during an ordinary allocation.

Mounting a ufs image needs CAP_SYS_ADMIN and ufs is not
unprivileged-mountable, so this is image-parsing hardening rather than a
privilege boundary, handled in the open like the recent sibling work:

  ufs: reject malformed cylinder summary geometry
    https://lore.kernel.org/all/20260701215700.822003-1-kudo3228@gmail.com/
  ufs: reject oversized cylinder group metadata
    https://lore.kernel.org/all/20260717104033.38574-1-david.lee@trailofbits.com/

Those validate fs_cssize and fs_cgsize in ufs_fill_super(); the checks
here live in ufs_read_cylinder() and do not overlap with them.

Both were reproduced under KASAN on an unpatched kernel by mounting a
crafted UFS2 image.

Patch 1, an image whose first cylinder group fails the magic check, so
ufs_read_cylinder_structures() fails after the root dentry is installed:

  BUG: KASAN: null-ptr-deref in mutex_lock+0x76/0xe0
   ufs_sync_fs+0x5b/0x6d0
  BUG: kernel NULL pointer dereference, address: 0000000000000100

Patch 2, an image whose on-disk cg_cgx is out of range, followed by a
write that allocates a block:

  BUG: KASAN: slab-out-of-bounds in adjust_free_blocks+0x1bf0/0x2510
   adjust_free_blocks

With the patches applied, patch 1 loads the cylinder groups before
publishing the root dentry and patch 2 rejects the mismatched cg_cgx in
ufs_read_cylinder(), so neither faulting access is reached.

Ali Ahmet Memis (2):
  ufs: create the root dentry after loading cylinder metadata
  ufs: validate cylinder group metadata before caching it

 fs/ufs/cylinder.c | 10 ++++++++++
 fs/ufs/super.c    | 17 +++++++++--------
 2 files changed, 19 insertions(+), 8 deletions(-)

-- 
2.54.0

[PATCH 1/2] ufs: create the root dentry after loading cylinder metadata

[Ali Ahmet Memis]

ufs_fill_super() installed sb->s_root before it loaded the cylinder
group structures for a writable mount:

	sb->s_root = d_make_root(inode);
	...
	if (!sb_rdonly(sb))
		if (!ufs_read_cylinder_structures(sb))
			goto failed;

When ufs_read_cylinder_structures() failed, the error path freed the
in-core superblock information and set sb->s_fs_info to NULL while
sb->s_root stayed installed. get_tree_bdev() then reached
deactivate_locked_super(), and because s_root was present,
generic_shutdown_super() called sync_filesystem() and the put_super
operation. Both dereference UFS_SB(sb), which is now NULL, so a mount
that fails only while reading the cylinder groups oopses during
teardown. A crafted image whose first cylinder group cannot be read
reaches this path.

Load the cylinder group metadata first and create the root dentry last,
so the superblock is published to the VFS only once it is fully set up.
ufs_setup_cstotal() and ufs_read_cylinder_structures() take only the
super_block and do not use the root inode, so the reordering is safe.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 fs/ufs/super.c | 17 +++++++++--------
 1 file changed, 9 insertions(+), 8 deletions(-)

diff --git a/fs/ufs/super.c b/fs/ufs/super.c
index c4831a8b9..7deecb395 100644
--- a/fs/ufs/super.c
+++ b/fs/ufs/super.c
@@ -1199,6 +1199,15 @@ static int ufs_fill_super(struct super_block *sb, struct fs_context *fc)
 	sb->s_maxbytes = ufs_max_bytes(sb);
 	sb->s_max_links = UFS_LINK_MAX;
 
+	ufs_setup_cstotal(sb);
+	/*
+	 * Read cylinder group structures
+	 */
+	if (!sb_rdonly(sb))
+		if (!ufs_read_cylinder_structures(sb))
+			goto failed;
+
+	/* create the root dentry last, once UFS_SB(sb) is fully set up */
 	inode = ufs_iget(sb, UFS_ROOTINO);
 	if (IS_ERR(inode)) {
 		ret = PTR_ERR(inode);
@@ -1210,14 +1219,6 @@ static int ufs_fill_super(struct super_block *sb, struct fs_context *fc)
 		goto failed;
 	}
 
-	ufs_setup_cstotal(sb);
-	/*
-	 * Read cylinder group structures
-	 */
-	if (!sb_rdonly(sb))
-		if (!ufs_read_cylinder_structures(sb))
-			goto failed;
-
 	UFSD("EXIT\n");
 	return 0;
 
-- 
2.54.0

Re: [PATCH 1/2] ufs: create the root dentry after loading cylinder metadata

[Jan Kara]

On Sat 01-08-26 10:12:57, Ali Ahmet Memis wrote:
> ufs_fill_super() installed sb->s_root before it loaded the cylinder
> group structures for a writable mount:
> 
> 	sb->s_root = d_make_root(inode);
> 	...
> 	if (!sb_rdonly(sb))
> 		if (!ufs_read_cylinder_structures(sb))
> 			goto failed;
> 
> When ufs_read_cylinder_structures() failed, the error path freed the
> in-core superblock information and set sb->s_fs_info to NULL while
> sb->s_root stayed installed. get_tree_bdev() then reached
> deactivate_locked_super(), and because s_root was present,
> generic_shutdown_super() called sync_filesystem() and the put_super
> operation. Both dereference UFS_SB(sb), which is now NULL, so a mount
> that fails only while reading the cylinder groups oopses during
> teardown. A crafted image whose first cylinder group cannot be read
> reaches this path.
> 
> Load the cylinder group metadata first and create the root dentry last,
> so the superblock is published to the VFS only once it is fully set up.
> ufs_setup_cstotal() and ufs_read_cylinder_structures() take only the
> super_block and do not use the root inode, so the reordering is safe.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ufs/super.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/ufs/super.c b/fs/ufs/super.c
> index c4831a8b9..7deecb395 100644
> --- a/fs/ufs/super.c
> +++ b/fs/ufs/super.c
> @@ -1199,6 +1199,15 @@ static int ufs_fill_super(struct super_block *sb, struct fs_context *fc)
>  	sb->s_maxbytes = ufs_max_bytes(sb);
>  	sb->s_max_links = UFS_LINK_MAX;
>  
> +	ufs_setup_cstotal(sb);
> +	/*
> +	 * Read cylinder group structures
> +	 */
> +	if (!sb_rdonly(sb))
> +		if (!ufs_read_cylinder_structures(sb))
> +			goto failed;
> +
> +	/* create the root dentry last, once UFS_SB(sb) is fully set up */
>  	inode = ufs_iget(sb, UFS_ROOTINO);
>  	if (IS_ERR(inode)) {
>  		ret = PTR_ERR(inode);
> @@ -1210,14 +1219,6 @@ static int ufs_fill_super(struct super_block *sb, struct fs_context *fc)
>  		goto failed;
>  	}
>  
> -	ufs_setup_cstotal(sb);
> -	/*
> -	 * Read cylinder group structures
> -	 */
> -	if (!sb_rdonly(sb))
> -		if (!ufs_read_cylinder_structures(sb))
> -			goto failed;
> -
>  	UFSD("EXIT\n");
>  	return 0;
>  
> -- 
> 2.54.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

[PATCH 2/2] ufs: validate cylinder group metadata before caching it

[Ali Ahmet Memis]

ufs_read_cylinder() copies the cylinder group index and the rotor
positions straight from the on-disk group and caches them without any
check:

	ucpi->c_cgx    = fs32_to_cpu(sb, ucg->cg_cgx);
	ucpi->c_rotor  = fs32_to_cpu(sb, ucg->cg_rotor);
	ucpi->c_frotor = fs32_to_cpu(sb, ucg->cg_frotor);
	ucpi->c_irotor = fs32_to_cpu(sb, ucg->cg_irotor);

They are then used as indices during allocation and free:

  - c_cgx indexes the cylinder summary array as
    UFS_SB(sb)->fs_cs(ucpi->c_cgx), so a value past s_ncg writes a 32
    bit count outside the s_csp allocation.

  - c_frotor becomes a bitmap scan start, start = c_frotor >> 3, and
    then length = ((s_fpg + 7) >> 3) - start. A start beyond the block
    bitmap wraps the unsigned length to a huge value, so ubh_scanc()
    walks far past the cylinder group buffers. c_irotor drives the
    inode bitmap the same way.

A crafted image can set any of these freely, turning an ordinary
allocation into an out of bounds access.

Reject a cylinder group whose recorded index does not match the group
being read, or whose rotors fall outside the group, before the metadata
is cached. Valid filesystems keep cg_cgx equal to the group number and
the rotors within the group, so only malformed images are rejected.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>
---
 fs/ufs/cylinder.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/fs/ufs/cylinder.c b/fs/ufs/cylinder.c
index a2813270c..b930ee1cf 100644
--- a/fs/ufs/cylinder.c
+++ b/fs/ufs/cylinder.c
@@ -68,6 +68,16 @@ static bool ufs_read_cylinder(struct super_block *sb,
 	ucpi->c_clustersumoff = fs32_to_cpu(sb, ucg->cg_u.cg_44.cg_clustersumoff);
 	ucpi->c_clusteroff = fs32_to_cpu(sb, ucg->cg_u.cg_44.cg_clusteroff);
 	ucpi->c_nclusterblks = fs32_to_cpu(sb, ucg->cg_u.cg_44.cg_nclusterblks);
+
+	/* these on-disk values become array and bitmap indices */
+	if (ucpi->c_cgx != cgno ||
+	    ucpi->c_rotor >= uspi->s_fpg ||
+	    ucpi->c_frotor >= uspi->s_fpg ||
+	    ucpi->c_irotor >= uspi->s_ipg) {
+		ufs_error(sb, __func__,
+			  "inconsistent metadata in cylinder group %u\n", cgno);
+		goto failed;
+	}
 	UFSD("EXIT\n");
 	return true;
 	
-- 
2.54.0

Re: [PATCH 2/2] ufs: validate cylinder group metadata before caching it

[Jan Kara]

On Sat 01-08-26 10:12:58, Ali Ahmet Memis wrote:
> ufs_read_cylinder() copies the cylinder group index and the rotor
> positions straight from the on-disk group and caches them without any
> check:
> 
> 	ucpi->c_cgx    = fs32_to_cpu(sb, ucg->cg_cgx);
> 	ucpi->c_rotor  = fs32_to_cpu(sb, ucg->cg_rotor);
> 	ucpi->c_frotor = fs32_to_cpu(sb, ucg->cg_frotor);
> 	ucpi->c_irotor = fs32_to_cpu(sb, ucg->cg_irotor);
> 
> They are then used as indices during allocation and free:
> 
>   - c_cgx indexes the cylinder summary array as
>     UFS_SB(sb)->fs_cs(ucpi->c_cgx), so a value past s_ncg writes a 32
>     bit count outside the s_csp allocation.
> 
>   - c_frotor becomes a bitmap scan start, start = c_frotor >> 3, and
>     then length = ((s_fpg + 7) >> 3) - start. A start beyond the block
>     bitmap wraps the unsigned length to a huge value, so ubh_scanc()
>     walks far past the cylinder group buffers. c_irotor drives the
>     inode bitmap the same way.
> 
> A crafted image can set any of these freely, turning an ordinary
> allocation into an out of bounds access.
> 
> Reject a cylinder group whose recorded index does not match the group
> being read, or whose rotors fall outside the group, before the metadata
> is cached. Valid filesystems keep cg_cgx equal to the group number and
> the rotors within the group, so only malformed images are rejected.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Ali Ahmet Memis <ali@iusegentoo.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ufs/cylinder.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/fs/ufs/cylinder.c b/fs/ufs/cylinder.c
> index a2813270c..b930ee1cf 100644
> --- a/fs/ufs/cylinder.c
> +++ b/fs/ufs/cylinder.c
> @@ -68,6 +68,16 @@ static bool ufs_read_cylinder(struct super_block *sb,
>  	ucpi->c_clustersumoff = fs32_to_cpu(sb, ucg->cg_u.cg_44.cg_clustersumoff);
>  	ucpi->c_clusteroff = fs32_to_cpu(sb, ucg->cg_u.cg_44.cg_clusteroff);
>  	ucpi->c_nclusterblks = fs32_to_cpu(sb, ucg->cg_u.cg_44.cg_nclusterblks);
> +
> +	/* these on-disk values become array and bitmap indices */
> +	if (ucpi->c_cgx != cgno ||
> +	    ucpi->c_rotor >= uspi->s_fpg ||
> +	    ucpi->c_frotor >= uspi->s_fpg ||
> +	    ucpi->c_irotor >= uspi->s_ipg) {
> +		ufs_error(sb, __func__,
> +			  "inconsistent metadata in cylinder group %u\n", cgno);
> +		goto failed;
> +	}
>  	UFSD("EXIT\n");
>  	return true;
>  	
> -- 
> 2.54.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH 0/2] ufs: harden the mount path against malformed images

[Christian Brauner]

On Sat, 01 Aug 2026 10:12:56 +0300, Ali Ahmet Memis wrote:
> Two robustness fixes for the ufs mount path, both reachable by mounting
> a crafted UFS image.
> 
> Patch 1 reorders ufs_fill_super() so the root dentry is created after the
> cylinder group metadata is loaded. Today a failure while reading the
> cylinder groups drops UFS_SB(sb) but leaves s_root installed, so the
> generic teardown oopses on a NULL pointer in ufs_sync_fs() and the
> put_super operation.
> 
> [...]

In the future, please cut down and rewrite LLM generated cover letters and
commit messages.

---

Applied to the vfs.fixes branch of the vfs/vfs.git tree.
Patches in the vfs.fixes branch should appear in linux-next soon.

Please report any outstanding bugs that were missed during review in a
new review to the original patch series allowing us to drop it.

It's encouraged to provide Acked-bys and Reviewed-bys even though the
patch has now been applied. If possible patch trailers will be updated.

Note that commit hashes shown below are subject to change due to rebase,
trailer updates or similar. If in doubt, please check the listed branch.

tree:   https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git
branch: vfs.fixes

[1/2] ufs: create the root dentry after loading cylinder metadata
      https://git.kernel.org/vfs/vfs/c/55a4c98abb96
[2/2] ufs: validate cylinder group metadata before caching it
      https://git.kernel.org/vfs/vfs/c/c9d263be2680

Re: [PATCH 0/2] ufs: harden the mount path against malformed images

[Ali Ahmet Memis]

On Tue, 25 Aug 2026, Christian Brauner wrote:
> In the future, please cut down and rewrite LLM generated cover letters and
> commit messages.

Thanks for applying these. I wrote them myself (English isn't my first language, so I guess that can make them sound a bit artificial) but I'll keep the
commit messages and cover letters shorter next time.