Skip to content
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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

flexibeast
Copy link

Further to this r/bedrocklinux discussion.

Tested in a QEMU VM; WorksForMe™.

stratum_root="/bedrock/strata/${stratum}"
for init in "${stratum_root}/bin/66" "${stratum_root}/usr/bin/66"; do
if [ -x ${init} ]; then
cp "${init}" "${init}-bedrock-patch"

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?

Copy link
Author

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:

  1. Create a copy of the file 66 for patching
  2. Use that copy as the destination for sed's patching
  3. Rename the copy as 66 once sed is done

This avoids leaving a broken 66 file if sed fails for some reason.

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.

@@ -343,6 +343,14 @@ if [ -r /etc/fstab ] && awk '$1$2$3$4$5$6 !~ "#" && $6 != "" && $6 != 0 {x=1} EN
' /etc/fstab >/etc/fstab-new && mv /etc/fstab-new /etc/fstab
fi

# When using 66 init, make a tmpfs available for 66's LIVE directory.
# The init_cmd variable is exported by bedrock init for use here.
Copy link
Member

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 to brl-apply, why not just do this operation in init directly? Mounting tmpfs is typically an init-time thing; I think most people reading the code base will expect it in init.

Copy link
Author

Choose a reason for hiding this comment

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

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.

nod Okay, good points.

Instead of trying to pass this information from init to brl-apply, why not just do this operation in init directly? Mounting tmpfs is typically an init-time thing; I think most people reading the code base will expect it in init.

Hmm, so around here, after the calls to brl-repair and brl-apply, so they don't clobber the 66-specific setup?

Copy link
Member

Choose a reason for hiding this comment

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

brl repair and brl 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:

  • Doing this before brl repair and brl 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 by brl repair and brl apply accordingly.
  • Doing this in a place that is already consider the init.

I'm thinking maybe here, but I'm not firm on that if you disagree.

Copy link
Member

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:

    char fs[livelen + 1] ;
    split_tmpfs(fs,live) ;
    r = is_mnt(fs) ;

    if (!r || tmpfs)
    {
        if (r && tmpfs)
        {
            log_info("Umount: ",fs) ;
            if (umount(fs) == -1) sulogin ("umount: ",fs ) ;
        }
        log_info("Mount: ",fs) ;
        if (mount("tmpfs", fs, "tmpfs", MS_NODEV | MS_NOSUID, "mode=0755") == -1)
            sulogin("mount: ",fs) ;
    }

Looks to me like it explicitly cares about LIVE's parent directory, not the LIVE directory itself.

mkdir -p $live_dir
mount -t tmpfs tmpfs $live_dir -o rw,nosuid,nodev,mode=755,exec
fi

Copy link
Member

@paradigm paradigm Oct 26, 2021

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:

  • Checking if /run is already mounted as desired, and if so, don't do anything.
  • If /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 in init, where I think this code should go anyways.

Copy link
Author

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.

Copy link
Member

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:

  • Go to the 66 folks and try to get them to change the default behavior to match your preference.
  • Go to the distros that distribute 66 and get them to change the default behavior to match your preference.
  • Make your own 66 distro.

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.

  • If a normal 66 system is picky about /run/66 being on a mount point with exec 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.
  • If a normal 66 system has /run/66 as a tmpfs mount point - irrelevant of /run - then we should do that.

Copy link
Author

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?

Copy link

@mobinmob mobinmob Oct 27, 2021

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

Copy link
Author

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?

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 :)

Copy link
Author

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.

Copy link
Author

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?

Copy link
Member

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:

  • Bedrock sets /run to have a certain mount property before handing control off to the specified init.
  • By default, 66 unmounts (default -m) anything found at /run (default LIVE) and re-mounts over it. This clears the mount property we want. This is problem (1) we need to solve.
  • By default, Bedrock mounts /run with -onoexec. However, per your experimentation 66 apparently requires /run (default LIVE) 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-vis exec or noexec. The proper way to do this would be to mount /etc/fstab directly in Bedrock's init 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 with awk and remount /run with exec/noexec in Bedrock's init.

When you wrote "the same point where it's doing other /etc/fstab stuff", i take it you mean the "# Configure /etc/fstab" section in brl-apply, and not the fstab stuff in installer.sh?

I meant Bedrock's init, the only place where it mounts /etc/fstab.

assume i'll have to check whether /run is mounted -oexec by default on Obarun and Artix?

When running 66 without any special patching regarding -m, yes, to confirm the theory that 66 requires LIVE / /run be -oexec.

Clearly if it doesn't, then we're barking up the wrong tree with problem (2).

And what do we do if the user has set LIVE to something outside of /run?

Require they update /etc/fstab accordingly.

src/slash-bedrock/libexec/brl-apply Show resolved Hide resolved
src/slash-bedrock/libexec/brl-apply Outdated Show resolved Hide resolved
src/slash-bedrock/libexec/brl-apply Outdated Show resolved Hide resolved
src/slash-bedrock/etc/bedrock.conf Show resolved Hide resolved
@flexibeast flexibeast force-pushed the 66-on-void branch 3 times, most recently from 5a198a2 to a9ed3a9 Compare October 28, 2021 07:54
@flexibeast
Copy link
Author

Unfortunately, i don't think i'm likely to get back to working on this PR any time soon - sorry. :-( i'm happy to keep this PR open should anyone wish to come along and pick up where i've left off - there's not a huge amount left to do - but equally, i'm happy for this to be closed if it's more clutter than anything else.

@paradigm
Copy link
Member

paradigm commented Mar 28, 2022

No worries. Bedrock's all volunteer effort when people have the time, and I certainly understand limitations there.

I do have some good news on this front. The difficulties here are, ultimately, due to a 0.7 design limitations which meant that the /run it sets up before handing control off to the init must be maintained. I think I can rework 0.8 to manage the information in such a fashion that this won't be a problem by making it so that any Bedrock content needed in /run will be stored elsewhere first and populated in /run after the init is done (re)mounting it. If we don't get this working in 0.7, I'm hopeful we will in 0.8.

@paradigm paradigm force-pushed the master branch 2 times, most recently from 12e1061 to 524beae Compare March 18, 2024 01:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants