Skip to content

fix: restrict EFI partition permissions with fmask/dmask=0077 #4506

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
0xdeadd wants to merge 3 commits into
base: master
Choose a base branch
from
+6 −1
Open

fix: restrict EFI partition permissions with fmask/dmask=0077 #4506

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
98cb263
fix: restrict EFI partition permissions with fmask/dmask=0077
0xdeadd Apr 30, 2026
6ed42e3
fix(efi): collapse fmask/dmask dedup to dict.fromkeys one-liner
0xdeadd May 13, 2026
3051cac
fix(efi): drop defensive list wrap per review
0xdeadd May 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion archinstall/lib/installer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,12 @@ def _mount_partition(self, part_mod: PartitionModification) -> None:
# it would be none if it's btrfs as the subvolumes will have the mountpoints defined
if part_mod.mountpoint:
target = self.target / part_mod.relative_mountpoint
mount(part_mod.dev_path, target, options=part_mod.mount_options)
options = part_mod.mount_options

if part_mod.is_efi():
options = list(dict.fromkeys(options + ['fmask=0077', 'dmask=0077']))
Copy link
Copy Markdown
Collaborator

@svartkanin svartkanin May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

change to options += ['fmask=0077', 'dmask=0077']

Copy link
Copy Markdown
Author

@0xdeadd 0xdeadd May 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The += form works, but it drops the dedup @Torxed asked for in his earlier comment. Two cases that motivated keeping it:

  1. If a disk config already sets fmask on the ESP (e.g. a profile with fmask=0022), += produces fmask=0022,fmask=0077 in /etc/fstab. vfat picks the last one so the result is still restrictive — but the line is untidy.
  2. On a re-install where fmask=0077 is already in mount_options, += writes it twice.

dict.fromkeys handles both cleanly while preserving insertion order. If the read is that both edge cases are too unlikely to be worth the extra logic, I'm fine simplifying. @Torxed, your call?

Copy link
Copy Markdown
Collaborator

@svartkanin svartkanin May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then use a set instead as it is cleaner. The entire mount_options should then probably be a set

Copy link
Copy Markdown
Author

@0xdeadd 0xdeadd May 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two reasons I'd lean toward keeping dict.fromkeys over a set here:

Order preservation. set iteration order in Python is implementation-defined, so two installs with identical mount-option inputs could emit entries in /etc/fstab in different orders. That's awkward for reproducibility and for diffing config across runs. dict.fromkeys dedups while preserving insertion order, which matches the semantics most callers expect from a list of mount options (and what was there before this PR).

Scope. Typing mount_options as set[str] (rather than just using a set locally) is reasonable on the merits, but it would touch PartitionModification and every serializer/deserializer that handles it — feels larger than this PR should carry. Happy to do it as a follow-up MR if you want, kept separate.

If you'd still prefer the local-set version (preserving the existing mount_options: list[str] type and just using a set inside _mount_partition), it's a one-line swap and I'll push it. Just say the word.


mount(part_mod.dev_path, target, options=options)
elif part_mod.fs_type == FilesystemType.BTRFS:
# Only mount BTRFS subvolumes that have mountpoints specified
subvols_with_mountpoints = [sv for sv in part_mod.btrfs_subvols if sv.mountpoint is not None]
Expand Down