* [PATCH v2] ufs: free cylinder group structures on late mount failure
2026-11-04 9:09 [PATCH] ufs: free cylinder group structures on late mount failure Joseph Qi
@ 2026-11-12 6:40 ` Joseph Qi
0 siblings, 0 replies; 2+ messages in thread
From: Joseph Qi @ 2026-11-12 6:40 UTC (permalink / raw)
To: Jan Kara, Christian Brauner; +Cc: linux-kernel, Joseph Qi, stable
ufs_fill_super() reads the cylinder group structures before it creates
the root dentry, and the two failures that remain after the read, a
failed ufs_iget() and a failed d_make_root(), jump to a label that only
frees the in-core superblock information. generic_shutdown_super()
calls put_super only inside its if (sb->s_root) block, so with s_root
left NULL the s_csp allocation, the s_ncg buffer heads behind s_ucg[]
and the UFS_MAX_GROUP_LOADED s_ucpi[] entries are leaked, once per
mount attempt. Neither failure needs an I/O error: ufs_iget() rejects
a root inode number outside the s_ncg * s_ipg range the image claims,
and s_ipg is not validated.
Move the unwind that ufs_read_cylinder_structures() already performs on
its own failure into ufs_free_cylinder_structures() and call it from
the two late mount failures as well.
The helper also clears everything it releases, which fixes a
stale-pointer bug on the remount path. A read-only remount runs
ufs_put_super_internal(), which used to free s_csp, s_ucg and s_ucpi[]
without clearing them, and a read-write remount then re-reads the
structures. If that re-read fails before reassigning the pointers,
the unwind frees the stale s_ucg and s_ucpi[] values a second time.
Have ufs_put_super_internal() flush the loaded cylinders and then call
the helper, so that every teardown clears what it frees, and reset
s_cg_loaded along with the pointers.
Fixes: 55a4c98abb96 ("ufs: create the root dentry after loading cylinder metadata")
Cc: stable@vger.kernel.org
Signed-off-by: Joseph Qi <joseph.qi@linux.alibaba.com>
---
fs/ufs/super.c | 72 +++++++++++++++++++++++++++++++-------------------
1 file changed, 45 insertions(+), 27 deletions(-)
diff --git a/fs/ufs/super.c b/fs/ufs/super.c
index 3569ac92b065b..7b0a9929e7594 100644
--- a/fs/ufs/super.c
+++ b/fs/ufs/super.c
@@ -448,6 +448,37 @@ static void ufs_setup_cstotal(struct super_block *sb)
UFSD("EXIT\n");
}
+/*
+ * Release everything ufs_read_cylinder_structures() allocated and clear
+ * everything released, so that no later error path can see a stale
+ * pointer: a read-only remount frees the structures through
+ * ufs_put_super_internal(), and a read-write remount re-reads them.
+ * Besides those two callers this is needed when a mount fails after the
+ * cylinder groups have been read: s_root is still NULL then, so
+ * generic_shutdown_super() never calls put_super and
+ * ufs_put_super_internal() does not run.
+ */
+static void ufs_free_cylinder_structures(struct super_block *sb)
+{
+ struct ufs_sb_info *sbi = UFS_SB(sb);
+ struct ufs_sb_private_info *uspi = sbi->s_uspi;
+ unsigned int i;
+
+ if (sbi->s_ucg) {
+ for (i = 0; i < uspi->s_ncg; i++)
+ brelse(sbi->s_ucg[i]);
+ kfree(sbi->s_ucg);
+ sbi->s_ucg = NULL;
+ for (i = 0; i < UFS_MAX_GROUP_LOADED; i++) {
+ kfree(sbi->s_ucpi[i]);
+ sbi->s_ucpi[i] = NULL;
+ }
+ }
+ sbi->s_cg_loaded = 0;
+ kfree(sbi->s_csp);
+ sbi->s_csp = NULL;
+}
+
/*
* Read on-disk structures associated with cylinder groups
*/
@@ -455,7 +486,7 @@ static int ufs_read_cylinder_structures(struct super_block *sb)
{
struct ufs_sb_info *sbi = UFS_SB(sb);
struct ufs_sb_private_info *uspi = sbi->s_uspi;
- unsigned char * base, * space;
+ unsigned char *space;
unsigned size, blks, i;
UFSD("ENTER\n");
@@ -466,9 +497,9 @@ static int ufs_read_cylinder_structures(struct super_block *sb)
*/
size = uspi->s_cssize;
blks = (size + uspi->s_fsize - 1) >> uspi->s_fshift;
- base = space = kmalloc(size, GFP_NOFS);
- if (!base)
- goto failed;
+ space = kmalloc(size, GFP_NOFS);
+ if (!space)
+ goto failed;
sbi->s_csp = (struct ufs_csum *)space;
for (i = 0; i < blks; i++) {
struct buffer_head *bh = sb_bread(sb, uspi->s_csaddr + i);
@@ -511,15 +542,7 @@ static int ufs_read_cylinder_structures(struct super_block *sb)
return 1;
failed:
- kfree (base);
- if (sbi->s_ucg) {
- for (i = 0; i < uspi->s_ncg; i++)
- if (sbi->s_ucg[i])
- brelse (sbi->s_ucg[i]);
- kfree (sbi->s_ucg);
- for (i = 0; i < UFS_MAX_GROUP_LOADED; i++)
- kfree (sbi->s_ucpi[i]);
- }
+ ufs_free_cylinder_structures(sb);
UFSD("EXIT (FAILED)\n");
return 0;
}
@@ -585,7 +608,7 @@ static void ufs_put_super_internal(struct super_block *sb)
{
struct ufs_sb_info *sbi = UFS_SB(sb);
struct ufs_sb_private_info *uspi = sbi->s_uspi;
- unsigned char * base, * space;
+ unsigned char *space;
unsigned blks, size, i;
@@ -594,7 +617,7 @@ static void ufs_put_super_internal(struct super_block *sb)
ufs_put_cstotal(sb);
size = uspi->s_cssize;
blks = (size + uspi->s_fsize - 1) >> uspi->s_fshift;
- base = space = (char*) sbi->s_csp;
+ space = (char *)sbi->s_csp;
for (i = 0; i < blks; i++, space += uspi->s_fsize) {
struct buffer_head *bh = sb_bread(sb, uspi->s_csaddr + i);
@@ -607,16 +630,9 @@ static void ufs_put_super_internal(struct super_block *sb)
mark_buffer_dirty(bh);
brelse(bh);
}
- for (i = 0; i < sbi->s_cg_loaded; i++) {
- ufs_put_cylinder (sb, i);
- kfree (sbi->s_ucpi[i]);
- }
- for (; i < UFS_MAX_GROUP_LOADED; i++)
- kfree (sbi->s_ucpi[i]);
- for (i = 0; i < uspi->s_ncg; i++)
- brelse (sbi->s_ucg[i]);
- kfree (sbi->s_ucg);
- kfree (base);
+ for (i = 0; i < sbi->s_cg_loaded; i++)
+ ufs_put_cylinder(sb, i);
+ ufs_free_cylinder_structures(sb);
UFSD("EXIT\n");
}
@@ -1211,17 +1227,19 @@ static int ufs_fill_super(struct super_block *sb, struct fs_context *fc)
inode = ufs_iget(sb, UFS_ROOTINO);
if (IS_ERR(inode)) {
ret = PTR_ERR(inode);
- goto failed;
+ goto failed_cg;
}
sb->s_root = d_make_root(inode);
if (!sb->s_root) {
ret = -ENOMEM;
- goto failed;
+ goto failed_cg;
}
UFSD("EXIT\n");
return 0;
+failed_cg:
+ ufs_free_cylinder_structures(sb);
failed:
if (ubh)
ubh_brelse_uspi (uspi);
--
2.39.3
^ permalink raw reply [flat|nested] 2+ messages in thread