Hi,
On 2026/1/6 10:55, Baolin Liu wrote:
From: Baolin Liu <[email protected]>
When updating fsid or domain_id, the code frees the old pointer before
allocating a new one. If allocation fails, the pointer becomes NULL
while the old value is already freed, causing state inconsistency.
Fix by allocating the new value first, and only freeing the old value
on success.
Signed-off-by: Baolin Liu <[email protected]>
---
fs/erofs/super.c | 18 ++++++++++++------
1 file changed, 12 insertions(+), 6 deletions(-)
diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index 937a215f626c..6e083d7e634c 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -509,16 +509,22 @@ static int erofs_fc_parse_param(struct fs_context *fc,
break;
#ifdef CONFIG_EROFS_FS_ONDEMAND
case Opt_fsid:
- kfree(sbi->fsid);
- sbi->fsid = kstrdup(param->string, GFP_KERNEL);
- if (!sbi->fsid)
+ char *new_fsid;
+
+ new_fsid = kstrdup(param->string, GFP_KERNEL);
May be there is no need to keep the old pointer. Because
1) The fsid/domain_id is ignored in reconfiguration.
2) Even if memory allocation fails when the user first mounts with multi
fsid/domain_id options (like -o fsid=xxx1,fsid=xxx2), the old fsid
pointer would also need to be released in cleanup procedure.
so am I right?
Thanks,
Hongbo
+ if (!new_fsid)
return -ENOMEM;
+ kfree(sbi->fsid);
+ sbi->fsid = new_fsid;
break;
case Opt_domain_id:
- kfree(sbi->domain_id);
- sbi->domain_id = kstrdup(param->string, GFP_KERNEL);
- if (!sbi->domain_id)
+ char *new_domain_id;
+
+ new_domain_id = kstrdup(param->string, GFP_KERNEL);
+ if (!new_domain_id)
return -ENOMEM;
+ kfree(sbi->domain_id);
+ sbi->domain_id = new_domain_id;
break;
#else
case Opt_fsid:
