Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.
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
Add support for 66. #239
base: master
Are you sure you want to change the base?
Add support for 66. #239
Changes from all commits
8cded7b
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't like this strategy. We can't rely on the assumption nothing overwrites
${init_cmd}
; it may be used elsewhere, e.g. by a user's shellrc file or something. It also unnecessarily pollutes the environment variable namespace.Instead of trying to pass this information from
init
tobrl-apply
, why not just do this operation ininit
directly? Mounting tmpfs is typically an init-time thing; I think most people reading the code base will expect it in init.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nod Okay, good points.
Hmm, so around here, after the calls to
brl-repair
andbrl-apply
, so they don't clobber the 66-specific setup?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
brl repair
andbrl apply
enforce Bedrock invariants. They essentially define what we want the system to be like. If there is concern that those clobber desired invariants, then something is wrong we need to fix. If you find those clobber your changes, we need to figure out why and reconsider our plan here.Assuming they don't clobber your changes, a couple desirable features for the location to apply the changes would be:
brl repair
andbrl apply
will make it a bit simpler, as there's fewer mount points to consider. So long as you're setting/changing the global/run
it should be adopted bybrl repair
andbrl apply
accordingly.I'm thinking maybe here, but I'm not firm on that if you disagree.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Poking around the source you linked elsewhere, I found this:
Looks to me like it explicitly cares about LIVE's parent directory, not the LIVE directory itself.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The default LIVE value appears to be
/run/66
./run
is commonly already a tmpfs mount point. Are you sure we want/run/66
to be another tmpfs mount point? While I haven't dug into it, I would think the expectation is to just use/run
's tmpfs.Check and confirm how 66 behaves with different
LIVE
values. Does 66 on a non-Bedrock setup actually have both/run
and/run/66
as mount points?If your goal here is just to get
/run
to have-oexec
, then consider:/run
is already mounted as desired, and if so, don't do anything./run
is already a tmpfs, but with the wrong options, use-o remount
to fix in-place.Bedrock already tries to enforce
/run
being a tmpfs ininit
, where I think this code should go anyways.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My thinking here was to keep the attack surface to a minimum. If only 66 needs
-oexec
, why make the entirety of the mount point-oexec
unnecessarily? But i'll certainly add the "check if/run
is already mounted as desired" logic in any case.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bedrock Linux, as a project, is very explicitly unopinionated. Its goal is to get features from other distros while making them feel as closely as we can to the distros they come from. Bedrock should only change what is needed to make features work. It's not Bedrock's job to be the judge of what is right or wrong about how features from other distros work. With regards to security, we should certainly minimize the new security concerns Bedrock itself adds; things like
strat
need to be secure. However, Bedrock shouldn't start messing with expectations from other distros, even in the name of security. If a user wants to do something horribly insecure, and the user requests it, that's on the user.If you have preferences on how a 66 distro should work, you're more than welcome to:
in which case it will be Bedrock's job to reflect the resulting effort as-is.
I've not played with 66 much. I'm trusting you to accurately reflect what it's actually like and not slip in your own preferences on how it should be like.
/run/66
being on a mount point withexec
and that it achieves this by mounting/run
as tmpfs with exec, and that's what a Bedrock user who requests 66 expects, than that's what we should do./run/66
as a tmpfs mount point - irrelevant of/run
- then we should do that.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
*nod* That all makes sense to me. i'm certainly not trying to impose my preferences overall; i'm just trying to do the minimum necessary to get things to work. In this instance, as i noted in this r/bedrocklinux comment, my experimentation found that
/run/66/
needed to be mounted-oexec
in order to avoid permission errors. It's in that context that i was trying to reduce how much of/run
was mounted-oexec
.Interestingly, when i look at the options for
/run
on my running Void+66 system, it's not mounted-oexec
. So on the face of it, it looks like something in the hijacking process is creating the need for-oexec
?More generally, i can't say i'm sure what a "normal 66 system" wants/expects re.
/run/66
; @mobinmob, can you offer any comments, or should i ask Eric?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
66-boot
is responsible for the livedir./run/66
is not hardcoded - almost nothing is in 66.I have changed some of the defaults for the voidlinux 66 package ;)
The livedir needs to be
rw
(not sure about exec...).https://web.obarun.org/software/66/latest/66-boot.html
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i just tried installing Artix (the suite66 version) and Obarun in QEMU VMs, without success - can't work out how to do a quick'n'dirty graphical-based install of Artix, and there were issues with refind-efi that blocked the Obarun install, i guess due to BIOS rather than EFI being the QEMU default; dunno if it's possible to change that. But in both instances, neither live system had
/run
mounted-noexec
, so maybe that suggests/run
in an installation would be-oexec
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is indeed possible..
It has been some time since I tested what actually happens on void without forcing remount in 66-boot. Maybe it is time some tests again :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mobinmob: In my testing, removing
-m
results in this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@paradigm: Ping re. my above questions?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@flexibeast
A summary of my understanding of the situation, to make sure we're on the same page:
/run
to have a certain mount property before handing control off to the specified init.-m
) anything found at/run
(defaultLIVE
) and re-mounts over it. This clears the mount property we want. This is problem (1) we need to solve./run
with-onoexec
. However, per your experimentation 66 apparently requires/run
(defaultLIVE
) to have-oexec
. This is problem (2) we need to solve.To solve (1), we force the removal of
-m
. As far as I recall, we haven't discussed an alternative.To solve (2), we want to respect user settings in
/etc/fstab
vis-a-visexec
ornoexec
. The proper way to do this would be to mount/etc/fstab
directly in Bedrock'sinit
before any other code to mount/run
. However, that may be tricky to do at this point in Poki's life without risk of breaking something. An alternative which may be good enough for this specific effort is to parse/etc/fstab
withawk
and remount/run
withexec/noexec
in Bedrock'sinit
.I meant Bedrock's
init
, the only place where it mounts/etc/fstab
.When running 66 without any special patching regarding
-m
, yes, to confirm the theory that 66 requiresLIVE
//run
be-oexec
.Clearly if it doesn't, then we're barking up the wrong tree with problem (2).
Require they update
/etc/fstab
accordingly.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does it need 66-bedrock-patch file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, i'm not sure what you're asking .... ? This hunk takes the same approach as the existing "Patch s6 init" code:
66
for patchingsed
's patching66
oncesed
is doneThis avoids leaving a broken
66
file ifsed
fails for some reason.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK, sed's patching, thx.