Re: [PATCH] erofs: set SB_NODEV sb_flags when mounting with fsid

From: Baokun Li
Date: Mon Apr 15 2024 - 10:08:38 EST


On 2024/4/15 21:38, Christian Brauner wrote:
On Mon, Apr 15, 2024 at 08:17:46PM +0800, Baokun Li wrote:
When erofs_kill_sb() is called in block dev based mode, s_bdev may not have
been initialised yet, and if CONFIG_EROFS_FS_ONDEMAND is enabled, it will
be mistaken for fscache mode, and then attempt to free an anon_dev that has
never been allocated, triggering the following warning:

============================================
ida_free called for id=0 which is not allocated.
WARNING: CPU: 14 PID: 926 at lib/idr.c:525 ida_free+0x134/0x140
Modules linked in:
CPU: 14 PID: 926 Comm: mount Not tainted 6.9.0-rc3-dirty #630
RIP: 0010:ida_free+0x134/0x140
Call Trace:
<TASK>
erofs_kill_sb+0x81/0x90
deactivate_locked_super+0x35/0x80
get_tree_bdev+0x136/0x1e0
vfs_get_tree+0x2c/0xf0
do_new_mount+0x190/0x2f0
[...]
============================================

To avoid this problem, add SB_NODEV to fc->sb_flags after successfully
parsing the fsid, and then the superblock inherits this flag when it is
allocated, so that the sb_flags can be used to distinguish whether it is
in block dev based mode when calling erofs_kill_sb().

Signed-off-by: Baokun Li <libaokun1@xxxxxxxxxx>
---
fs/erofs/super.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/fs/erofs/super.c b/fs/erofs/super.c
index b21bd8f78dc1..7539ce7d64bc 100644
--- a/fs/erofs/super.c
+++ b/fs/erofs/super.c
@@ -520,6 +520,7 @@ static int erofs_fc_parse_param(struct fs_context *fc,
ctx->fsid = kstrdup(param->string, GFP_KERNEL);
if (!ctx->fsid)
return -ENOMEM;
+ fc->sb_flags |= SB_NODEV;
Hm, I wouldn't do it this way. That's an abuse of that flag imho.
Record the information in the erofs_fs_context if you need to.
Hi Christian!

The problem here is that when mounting erofs, if we have an fsid
then it is not block device based, if we don't have an fsid it is block
device based. So only after we confirmed whether we have an fsid
or not, we can confirm whether we need SB_NODEV or not.

--
With Best Regards,
Baokun Li
.