Dear Hongbo Li,
I have reviewed this carefully, and I agree with your point. The old value will
eventually be freed in erofs_sb_free(), and keeping it here does not appear to
be necessary. Therefore, this patch does not need to be considered further.
Thank you for your review.
Dear Gao Xiang,
Thank you for your review as well.
Best regards,
Baolin Liu
在 2026/1/6 11:30, Hongbo Li 写道:
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:
