Re: [PATCH] debugfs show actual source in /proc/mounts

[Marc Aurèle La France]

On Fri, 2024-Aug-30, Eric Sandeen wrote:
> On 8/29/24 10:44 PM, Marc Aurèle La France wrote:
> > After commit 0c07c273a5fe ("debugfs: continue to ignore unknown mount
> > options"), debugfs displays "none" in /proc/mounts instead of the actual
> > source.  Fix this by recognising its "source" mount option.

> > Signed-off-by: Marc Aurèle La France <tsi@xxxxxxxxxx>
> > Fixes: 0c07c273a5fe ("debugfs: continue to ignore unknown mount options")
> > Cc: stable@xxxxxxxxxxxxxxx # 6.10.x: 9f111059e725: fs_parse: add uid & gid option option parsing helpers
> > Cc: stable@xxxxxxxxxxxxxxx # 6.10.x: 49abee5991e1: debugfs: Convert to new uid/gid option parsing helpers

> > diff -NRapruz -X /etc/diff.excludes linux-6.11.0-rc2/fs/debugfs/inode.c devel-6.11.0-rc2/fs/debugfs/inode.c
> > --- linux-6.11.0-rc5/fs/debugfs/inode.c
> > +++ devel-6.11.0-rc5/fs/debugfs/inode.c
> > @@ -89,12 +89,14 @@ enum {
> >  	Opt_uid,
> >  	Opt_gid,
> >  	Opt_mode,
> > +	Opt_source,
> >  };
> >
> >  static const struct fs_parameter_spec debugfs_param_specs[] = {
> >  	fsparam_gid	("gid",		Opt_gid),
> >  	fsparam_u32oct	("mode",	Opt_mode),
> >  	fsparam_uid	("uid",		Opt_uid),
> > +	fsparam_string	("source",	Opt_source),
> >  	{}
> >  };
> >
> > @@ -126,6 +128,12 @@ static int debugfs_parse_param(struct fs_context *fc, struct fs_parameter *param)
> >  	case Opt_mode:
> >  		opts->mode = result.uint_32 & S_IALLUGO;
> >  		break;

> Sorry for missing your earlier question, I was thinking that perhaps a
> comment along the lines of this would be helpful:

> 	/*
> 	 * Because debugfs accepts all mount options and indicates
> 	 * success even for unknown options, we must process "source"
> 	 * ourselves here; the vfs won't do it for us.
> 	 */

I disagree.  It is the new mount API that treats the source as a mount
"option", so to speak, despite what `man mount` says.  Therefore the
consequences of that change should be documented in the API, not here nor
in any other fs.  Besides, there's already too much echo in the comments
around this code.

> > +	case Opt_source:
> > +		if (fc->source)
> > +			return invalfc(fc, "Multiple sources specified");
> > +		fc->source = param->string;
> > +		param->string = NULL;
> > +		break;

> but I suppose it's not a big deal unless others think it is.

> Reviewed-by: Eric Sandeen <sandeen@xxxxxxxxxx>

Thanks.

Marc.