On 5/24/24 8:55 AM, Christian Brauner wrote:
> On Wed, May 22, 2024 at 10:38:51AM +0200, Wolfram Sang wrote:
>> The 'noauto' and 'auto' options were missed when migrating to the new
>> mount API. As a result, users with these in their fstab mount options
>> are now unable to mount debugfs filesystems, as they'll receive an
>> "Unknown parameter" error.
>>
>> This restores the old behaviour of ignoring noauto and auto if they're
>> given.
>>
>> Fixes: a20971c18752 ("vfs: Convert debugfs to use the new mount API")
>> Signed-off-by: Wolfram Sang <wsa+renesas@xxxxxxxxxxxxxxxxxxxx>
>> ---
>>
>> With current top-of-tree, debugfs remained empty on my boards triggering
>> the message "debugfs: Unknown parameter 'auto'". I applied a similar fix
>> which CIFS got and largely reused the commit message from 19d51588125f
>> ("cifs: ignore auto and noauto options if given").
>>
>> Given the comment in debugfs_parse_param(), I am not sure if this patch
>> is a complete fix or if there are more options to be ignored. This patch
>> makes it work for me(tm), however.
>>
>> From my light research, tracefs (which was converted to new mount API
>> together with debugfs) doesn't need the same fixing. But I am not
>> super-sure about that.
>
> Afaict, the "auto" option has either never existent or it was removed before
> the new mount api conversion time ago for debugfs. In any case, the root of the
> issue is that we used to ignore unknown mount options in the old mount api so
> you could pass anything that you would've wanted in there:
>
> /*
> * We might like to report bad mount options here;
> * but traditionally debugfs has ignored all mount options
> */
>
> So there's two ways to fix this:
>
> (1) We continue ignoring mount options completely when they're coming
> from the new mount api.
> (2) We continue ignoring mount options toto caelo.
>
> The advantage of (1) is that we gain the liberty to report errors to
> users on unknown mount options in the future but it will break on
> mount(8) from util-linux that relies on the new mount api by default. So
> I think what we need is (2) so something like:
Argh, sorry I missed this thread until now.
FWIW, I think the "ignore unknown mount options" was a weird old artifact;
unknown options were only ignored originally because there were none at all,
hence no parser to reject anything.
Still, it seems odd to me that "auto/noauto" were ever passed to the kernel,
I thought those were just a hint to userspace mount tools, no?
And why wouldn't every other filesystem with rejection of unknown options
fail in the same way?
And indeed, if I add this line to my fstab on a fedora rawhide box with the
latest upstream kernel that has the new mount API debugfs patch present
debugfs /debugfs-test debugfs auto 0 0
and strace "mount -a" or "mount /debugfs-test" I do not see any "auto" options
being passed to the kernel, and both commands succeed happily. Same if I change
"auto" to "noauto"
@Wolfram, what did your actual fstab line look like? I wonder what is actually
trying to pass auto as a mount option, and why...
-Eric
> diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
> index dc51df0b118d..713b6f76e75d 100644
> --- a/fs/debugfs/inode.c
> +++ b/fs/debugfs/inode.c
> @@ -107,8 +107,16 @@ static int debugfs_parse_param(struct fs_context *fc, struct fs_parameter *param
> int opt;
>
> opt = fs_parse(fc, debugfs_param_specs, param, &result);
> - if (opt < 0)
> + if (opt < 0) {
> + /*
> + * We might like to report bad mount options here; but
> + * traditionally debugfs has ignored all mount options
> + */
> + if (opt == -ENOPARAM)
> + return 0;
> +
> return opt;
> + }
>
> switch (opt) {
> case Opt_uid:
>
>
> Does that fix it for you?
>
