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

Debbugs page

Jonathan Brielmaier wrote 3 years ago
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
Maxim Cournoyer wrote 3 years ago
(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
Ludovic Courtès wrote 3 years ago
(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’.
Tobias Geerinckx-Rice wrote 3 years ago
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-----

Maxim Cournoyer wrote 3 years ago
(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
Tobias Geerinckx-Rice wrote 3 years ago
(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-----

Maxim Cournoyer wrote 3 years ago
(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
Ludovic Courtès wrote 3 years ago
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
Ludovic Courtès wrote 3 years ago
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
You may also tag this issue. See list of standard tags. For example, to set the confirmed and easy tags
mumi command -t +confirmed -t +easy
Or, remove the moreinfo tag and set the help tag
mumi command -t -moreinfo -t +help