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 <[email protected]>
---
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.
Looking forward to comments.
fs/debugfs/inode.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index dc51df0b118d..915f0b618486 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -89,12 +89,14 @@ enum {
Opt_uid,
Opt_gid,
Opt_mode,
+ Opt_ignore,
};
static const struct fs_parameter_spec debugfs_param_specs[] = {
fsparam_u32 ("gid", Opt_gid),
fsparam_u32oct ("mode", Opt_mode),
fsparam_u32 ("uid", Opt_uid),
+ fsparam_flag_no ("auto", Opt_ignore),
{}
};
--
2.39.2
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 <[email protected]>
> ---
>
> 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:
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?
Hi Christian,
> Afaict, the "auto" option has either never existent or it was removed before
> the new mount api conversion time ago for debugfs.
Frankly, I have no idea why I put this 'auto' in my fstab ages ago. But
it seems, I am not the only one[1].
[1] https://www.ibm.com/docs/en/linux-on-systems?topic=assumptions-debugfs
> 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?
Yes, it does, thank you.
Reported-by: Wolfram Sang <[email protected]>
Tested-by: Wolfram Sang <[email protected]>
Happy hacking,
Wolfram
Hi Wolfram,
On Mon, May 27, 2024 at 12:08 PM Wolfram Sang
<[email protected]> wrote:
> > Afaict, the "auto" option has either never existent or it was removed before
> > the new mount api conversion time ago for debugfs.
>
> Frankly, I have no idea why I put this 'auto' in my fstab ages ago. But
> it seems, I am not the only one[1].
fstab(5):
defaults
use default options: rw, suid, dev, exec, auto, nouser, and async.
noauto
do not mount when mount -a is given (e.g., at boot time)
So I assume "auto" is still passed when using "defaults"?
However, nowadays (since +10y?), debugfs etc. tend to no longer be
put in /etc/fstab, but be mounted automatically by some initscript.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68korg
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
On Mon, May 27, 2024 at 12:06:18PM +0200, Wolfram Sang wrote:
> Hi Christian,
>
> > Afaict, the "auto" option has either never existent or it was removed before
> > the new mount api conversion time ago for debugfs.
>
> Frankly, I have no idea why I put this 'auto' in my fstab ages ago. But
> it seems, I am not the only one[1].
>
> [1] https://www.ibm.com/docs/en/linux-on-systems?topic=assumptions-debugfs
>
> > 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?
>
> Yes, it does, thank you.
>
> Reported-by: Wolfram Sang <[email protected]>
> Tested-by: Wolfram Sang <[email protected]>
Thanks, applied. Should be fixed by end of the week.
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 <[email protected]>
>> ---
>>
>> 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?
>
Hi Eric,
thanks for replying!
> @Wolfram, what did your actual fstab line look like? I wonder what is actually
> trying to pass auto as a mount option, and why...
My fstab entry looks like this:
debugfs /sys/kernel/debug debugfs auto 0 0
This happened on an embedded system using busybox 1.33.0. strace is
currently not installed but I can try adding it if that is needed.
Happy hacking,
Wolfram
> > > Does that fix it for you?
> >
> > Yes, it does, thank you.
> >
> > Reported-by: Wolfram Sang <[email protected]>
> > Tested-by: Wolfram Sang <[email protected]>
>
> Thanks, applied. Should be fixed by end of the week.
It is in -next but not in rc2. rc3 then?
On Mon, Jun 03, 2024 at 09:24:50AM +0200, Wolfram Sang wrote:
>
> > > > Does that fix it for you?
> > >
> > > Yes, it does, thank you.
> > >
> > > Reported-by: Wolfram Sang <[email protected]>
> > > Tested-by: Wolfram Sang <[email protected]>
> >
> > Thanks, applied. Should be fixed by end of the week.
>
> It is in -next but not in rc2. rc3 then?
Yes, it wasn't ready when I sent the fixes for -rc2 as I just put it in
that day.
On 6/3/24 8:31 AM, Christian Brauner wrote:
> On Mon, Jun 03, 2024 at 09:24:50AM +0200, Wolfram Sang wrote:
>>
>>>>> Does that fix it for you?
>>>>
>>>> Yes, it does, thank you.
>>>>
>>>> Reported-by: Wolfram Sang <[email protected]>
>>>> Tested-by: Wolfram Sang <[email protected]>
>>>
>>> Thanks, applied. Should be fixed by end of the week.
>>
>> It is in -next but not in rc2. rc3 then?
>
> Yes, it wasn't ready when I sent the fixes for -rc2 as I just put it in
> that day.
>
See my other reply, are you sure we should make this change? From a
"keep the old behavior" POV maybe so, but this looks to me like a
bug in busybox, passing fstab hint "options" like "auto" as actual mount
options being the root cause of the problem. debugfs isn't uniquely
affected by this behavior.
I'm not dead set against the change, just wanted to point this out.
Thanks,
-Eric
On Mon, Jun 03, 2024 at 09:17:10AM -0500, Eric Sandeen wrote:
> On 6/3/24 8:31 AM, Christian Brauner wrote:
> > On Mon, Jun 03, 2024 at 09:24:50AM +0200, Wolfram Sang wrote:
> >>
> >>>>> Does that fix it for you?
> >>>>
> >>>> Yes, it does, thank you.
> >>>>
> >>>> Reported-by: Wolfram Sang <[email protected]>
> >>>> Tested-by: Wolfram Sang <[email protected]>
> >>>
> >>> Thanks, applied. Should be fixed by end of the week.
> >>
> >> It is in -next but not in rc2. rc3 then?
> >
> > Yes, it wasn't ready when I sent the fixes for -rc2 as I just put it in
> > that day.
> >
>
> See my other reply, are you sure we should make this change? From a
> "keep the old behavior" POV maybe so, but this looks to me like a
> bug in busybox, passing fstab hint "options" like "auto" as actual mount
> options being the root cause of the problem. debugfs isn't uniquely
> affected by this behavior.
>
> I'm not dead set against the change, just wanted to point this out.
Hm, it seems I forgot your other mail, sorry.
So the issue is that we're breaking existing userspace and it doesn't
seem like a situation where we can just ignore broken userspace. If
busybox has been doing that for a long time we might just have to
accommodate their brokenness. Thoughts?
On 6/3/24 9:33 AM, Christian Brauner wrote:
> On Mon, Jun 03, 2024 at 09:17:10AM -0500, Eric Sandeen wrote:
>> On 6/3/24 8:31 AM, Christian Brauner wrote:
>>> On Mon, Jun 03, 2024 at 09:24:50AM +0200, Wolfram Sang wrote:
>>>>
>>>>>>> Does that fix it for you?
>>>>>>
>>>>>> Yes, it does, thank you.
>>>>>>
>>>>>> Reported-by: Wolfram Sang <[email protected]>
>>>>>> Tested-by: Wolfram Sang <[email protected]>
>>>>>
>>>>> Thanks, applied. Should be fixed by end of the week.
>>>>
>>>> It is in -next but not in rc2. rc3 then?
>>>
>>> Yes, it wasn't ready when I sent the fixes for -rc2 as I just put it in
>>> that day.
>>>
>>
>> See my other reply, are you sure we should make this change? From a
>> "keep the old behavior" POV maybe so, but this looks to me like a
>> bug in busybox, passing fstab hint "options" like "auto" as actual mount
>> options being the root cause of the problem. debugfs isn't uniquely
>> affected by this behavior.
>>
>> I'm not dead set against the change, just wanted to point this out.
>
> Hm, it seems I forgot your other mail, sorry.
No worries!
> So the issue is that we're breaking existing userspace and it doesn't
> seem like a situation where we can just ignore broken userspace. If
> busybox has been doing that for a long time we might just have to
> accommodate their brokenness. Thoughts?
Yep, I can totally see that POV.
It's just that surely every other strict-parsing filesystem is also
broken in this same way, so coding around the busybox bug only in debugfs
seems a little strange. (Surely we won't change every filesystem to accept
unknown options just for busybox's benefit.)
IOWS: why do we accomodate busybox brokenness only for debugfs, given that
"auto" can be used in fstab for any filesystem?
But in simplest terms - it was, in fact, debugfs that a) changed and
b) got the bug report, so I don't have strong objections to going back
to the old behavior.
-Eric
> See my other reply, are you sure we should make this change? From a
> "keep the old behavior" POV maybe so, but this looks to me like a
> bug in busybox, passing fstab hint "options" like "auto" as actual mount
> options being the root cause of the problem. debugfs isn't uniquely
> affected by this behavior.
For the record, I plan to fix busybox. However, there are still a lot of
old versions around...
On Mon, Jun 03, 2024 at 10:13:43AM -0500, Eric Sandeen wrote:
> On 6/3/24 9:33 AM, Christian Brauner wrote:
> > On Mon, Jun 03, 2024 at 09:17:10AM -0500, Eric Sandeen wrote:
> >> On 6/3/24 8:31 AM, Christian Brauner wrote:
> >>> On Mon, Jun 03, 2024 at 09:24:50AM +0200, Wolfram Sang wrote:
> >>>>
> >>>>>>> Does that fix it for you?
> >>>>>>
> >>>>>> Yes, it does, thank you.
> >>>>>>
> >>>>>> Reported-by: Wolfram Sang <[email protected]>
> >>>>>> Tested-by: Wolfram Sang <[email protected]>
> >>>>>
> >>>>> Thanks, applied. Should be fixed by end of the week.
> >>>>
> >>>> It is in -next but not in rc2. rc3 then?
> >>>
> >>> Yes, it wasn't ready when I sent the fixes for -rc2 as I just put it in
> >>> that day.
> >>>
> >>
> >> See my other reply, are you sure we should make this change? From a
> >> "keep the old behavior" POV maybe so, but this looks to me like a
> >> bug in busybox, passing fstab hint "options" like "auto" as actual mount
> >> options being the root cause of the problem. debugfs isn't uniquely
> >> affected by this behavior.
> >>
> >> I'm not dead set against the change, just wanted to point this out.
> >
> > Hm, it seems I forgot your other mail, sorry.
>
> No worries!
>
> > So the issue is that we're breaking existing userspace and it doesn't
> > seem like a situation where we can just ignore broken userspace. If
> > busybox has been doing that for a long time we might just have to
> > accommodate their brokenness. Thoughts?
>
> Yep, I can totally see that POV.
>
> It's just that surely every other strict-parsing filesystem is also
> broken in this same way, so coding around the busybox bug only in debugfs
> seems a little strange. (Surely we won't change every filesystem to accept
> unknown options just for busybox's benefit.)
>
> IOWS: why do we accomodate busybox brokenness only for debugfs, given that
> "auto" can be used in fstab for any filesystem?
I suspect that not that most filesystems aren't mounted from fstab which
is why we've never saw reports.