Invalid <file-system> flags lead to a crash when booting

  • Done
  • quality assurance status badge
Details
4 participants
  • Jonathan Brielmaier
  • Ludovic Courtès
  • Maxim Cournoyer
  • Tobias Geerinckx-Rice
Owner
unassigned
Submitted by
Jonathan Brielmaier
Severity
normal
J
J
Jonathan Brielmaier wrote on 27 Oct 2021 00:24
file-system: validate flags
(address . bug-guix@gnu.org)
33625107-5cf9-85f8-9025-2b9f186723bc@web.de
Imagine the following file system definition in your config.scm:

```
(file-system
(device (uuid "UUID-123"))
(flags '((create-mount-point? #t)))
(mount-point "/mnt")
(type "ext4")))
```

When you reconfigure there will be no complain, but when you reboot your
system wont boot. The parameter to flags is nonsense, it should be
something like: `read-only`. So mounting of the file system will fail...

It would be nice if we can have some flag validation during reconfigure.

~Jonathan
M
M
Maxim Cournoyer wrote on 28 Oct 2021 03:31
(name . Jonathan Brielmaier)(address . jonathan.brielmaier@web.de)(address . 51425@debbugs.gnu.org)
87zgqu9bpk.fsf@gmail.com
Hello,

Jonathan Brielmaier <jonathan.brielmaier@web.de> writes:

Toggle quote (18 lines)
> Imagine the following file system definition in your config.scm:
>
> ```
> (file-system
> (device (uuid "UUID-123"))
> (flags '((create-mount-point? #t)))
> (mount-point "/mnt")
> (type "ext4")))
> ```
>
> When you reconfigure there will be no complain, but when you reboot your
> system wont boot. The parameter to flags is nonsense, it should be
> something like: `read-only`. So mounting of the file system will fail...
>
> It would be nice if we can have some flag validation during reconfigure.
>
> ~Jonathan

I agree that it'd be nice; I had gotten close to implementing such a
thing in the past, but it was discussed and abandoned because each file
system may have their own flags, add new flags with new releases, etc,
which means it'd be difficult to keep the list accurate.

That's if my memory serves me right :-).

Thanks,

Maxim
L
L
Ludovic Courtès wrote on 29 Oct 2021 21:13
(name . Jonathan Brielmaier)(address . jonathan.brielmaier@web.de)
87cznny78b.fsf@gnu.org
Hi!

Jonathan Brielmaier <jonathan.brielmaier@web.de> skribis:

Toggle quote (13 lines)
> Imagine the following file system definition in your config.scm:
>
> ```
> (file-system
> (device (uuid "UUID-123"))
> (flags '((create-mount-point? #t)))
> (mount-point "/mnt")
> (type "ext4")))
> ```
>
> When you reconfigure there will be no complain, but when you reboot your
> system wont boot.

I suppose it fails to boot because of a match error in
‘mount-flags->bit-mask’, right?

Toggle quote (3 lines)
> The parameter to flags is nonsense, it should be something like:
> `read-only`. So mounting of the file system will fail...

That’s a good use case for the recently-added ‘sanitize’ record field
property.

Ludo’.
T
T
Tobias Geerinckx-Rice wrote on 29 Oct 2021 21:41
87lf2bipgp.fsf@nckx
[FTR, I'm the one who encouraged Jonathan to file a bug. Mount
flags seem ideally suited to build-time validation.]

Maxim,

Maxim Cournoyer ???
Toggle quote (2 lines)
> each file system may have their own flags

Really? Huh. Interesting. How does that work? Where would
these file-system specific flags be #defined?

(My guess is that if anyone does this, it'd be ZFS ;-)

Toggle quote (2 lines)
> add new flags with new [Linux] releases

Well, sure, but then we simply add them to Guix. This applies to
everything from syscalls to services.

Ludo', I'll take a look at ‘sanitize’; thanks!

Kind regards,

T G-R
-----BEGIN PGP SIGNATURE-----

iIMEARYKACsWIQT12iAyS4c9C3o4dnINsP+IT1VteQUCYXxPlw0cbWVAdG9iaWFz
LmdyAAoJEA2w/4hPVW15SgYA/0Qd9jtqLWTcopDd6QU3WruHqjmQnS6kb8HtrAlk
tbAYAP4yPvTlVGJ+o5rKGgKr2Nm92R38XS72zQ1wJolDqQkECg==
=R3T3
-----END PGP SIGNATURE-----

M
M
Maxim Cournoyer wrote on 30 Oct 2021 02:45
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)
87fssj8hnb.fsf@gmail.com
Hello Tobias,

Tobias Geerinckx-Rice <me@tobias.gr> writes:

Toggle quote (11 lines)
> [FTR, I'm the one who encouraged Jonathan to file a bug. Mount flags
> seem ideally suited to build-time validation.]
>
> Maxim,
>
> Maxim Cournoyer ???
>> each file system may have their own flags
>
> Really? Huh. Interesting. How does that work? Where would these
> file-system specific flags be #defined?

Or maybe I'm confusing with file system options; I can't find the past
discussion that I had on mind; but options are file driver specific,
while flags are file system independent. So yes, flags are a good
candidate for validation!

Another thing that is tricky about options is that some of them are only
really understood by the 'mount' command line utility, not the 'mount'
system call such as used in our init RAM disk (both are thrown together
in 'man 8 mount' without an easy way to discern them apart, IIRC).

Toggle quote (2 lines)
> Ludo', I'll take a look at ‘sanitize’; thanks!

Neat, thank you Tobias!

Maxim
T
T
Tobias Geerinckx-Rice wrote on 30 Oct 2021 02:48
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)
878rybib00.fsf@nckx
Maxim,

Maxim Cournoyer ???
Toggle quote (9 lines)
> Another thing that is tricky about options is that some of them
> are only
> really understood by the 'mount' command line utility, not the
> 'mount'
> system call such as used in our init RAM disk (both are thrown
> together
> in 'man 8 mount' without an easy way to discern them apart,
> IIRC).

Aha! Let me introduce you to the only slightly unfortunately
named ‘man 2 mount’ instead.

The way mount(8) lumps both together is… I guess it's
user-friendly — in a way? — but it leads to this total confusion
about what's what. Think of flags as literal bit flags, mainly
because they are.

Guix does make the distinction. It's the right call but it leads
to a brief education moment the first time you encounter both
fields.

You're absolutely right that mount options OTOH are arbitrary
strings. They can't and shouldn't be ‘validated’, but we don't
currently mandate their stringiness, and should.

Kind regards,

T G-R
-----BEGIN PGP SIGNATURE-----

iIMEARYKACsWIQT12iAyS4c9C3o4dnINsP+IT1VteQUCYXyYzw0cbWVAdG9iaWFz
LmdyAAoJEA2w/4hPVW15IsABAMgyDMpFX7Q1THfIQ0vOxqXiSO0Z8v8OmQr4860H
DKKhAPwNyHO4B0sD925g8YqVpB1/0todcP0NEF8kUSRqYdEeAg==
=y72O
-----END PGP SIGNATURE-----

M
M
Maxim Cournoyer wrote on 31 Oct 2021 03:31
(name . Tobias Geerinckx-Rice)(address . me@tobias.gr)
87zgqp7wmr.fsf@gmail.com
Hi Tobias,

Tobias Geerinckx-Rice <me@tobias.gr> writes:

Toggle quote (14 lines)
> Maxim,
>
> Maxim Cournoyer ???
>> Another thing that is tricky about options is that some of them are
>> only
>> really understood by the 'mount' command line utility, not the
>> 'mount'
>> system call such as used in our init RAM disk (both are thrown
>> together
>> in 'man 8 mount' without an easy way to discern them apart, IIRC).
>
> Aha! Let me introduce you to the only slightly unfortunately named
> ‘man 2 mount’ instead.

Eh, I'm not sure why I hadn't thought about that myself, thank you! It
makes sense after all -- one man page (2) documents the options for the
system call, the other one (8) for the command.

Toggle quote (4 lines)
> The way mount(8) lumps both together is… I guess it's user-friendly —
> in a way? — but it leads to this total confusion about what's what.
> Think of flags as literal bit flags, mainly because they are.

Hehe.

Toggle quote (7 lines)
> Guix does make the distinction. It's the right call but it leads to a
> brief education moment the first time you encounter both fields.
>
> You're absolutely right that mount options OTOH are arbitrary strings.
> They can't and shouldn't be ‘validated’, but we don't currently
> mandate their stringiness, and should.

Makes sense.

Thank you!

Maxim
L
L
Ludovic Courtès wrote on 7 Nov 2021 23:15
control message for bug #51425
(address . control@debbugs.gnu.org)
87cznbk3xx.fsf@gnu.org
retitle 51425 Invalid <file-system> flags lead to a crash when booting
quit
L
L
Ludovic Courtès wrote on 7 Nov 2021 23:16
Re: bug#51425: file-system: validate flags
(name . Jonathan Brielmaier)(address . jonathan.brielmaier@web.de)
878rxzk3wu.fsf@gnu.org
Hi,

Ludovic Courtès <ludo@gnu.org> skribis:

Toggle quote (24 lines)
> Jonathan Brielmaier <jonathan.brielmaier@web.de> skribis:
>
>> Imagine the following file system definition in your config.scm:
>>
>> ```
>> (file-system
>> (device (uuid "UUID-123"))
>> (flags '((create-mount-point? #t)))
>> (mount-point "/mnt")
>> (type "ext4")))
>> ```
>>
>> When you reconfigure there will be no complain, but when you reboot your
>> system wont boot.
>
> I suppose it fails to boot because of a match error in
> ‘mount-flags->bit-mask’, right?
>
>> The parameter to flags is nonsense, it should be something like:
>> `read-only`. So mounting of the file system will fail...
>
> That’s a good use case for the recently-added ‘sanitize’ record field
> property.

Done in 5eb5c0789f34e87ee417a53ddfcfa3b6521bb337.

Thanks,
Ludo’.
Closed
?
Your comment

This issue is archived.

To comment on this conversation send an email to 51425@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 51425
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch