> Date: Thu, 21 Sep 2023 22:30:01 +0000
> From: Klemens Nanni <k...@openbsd.org>
>
> In comparison to MI boot which only cares about /bsd.upgrade's x bit,
> powerpc64 rdboot just wants a regular file.
>
> Require and strip u+x before execution to prevent sysupgrade(8) loop.
> I'm new to powerpc64 and can't think of a reason to be different.
>
> Feedback? Objection? OK?
So there is a problem with this approach. Calling chmod() will mean
the bootloader will change the filesystem. What happens if you're
booting with a root filesystem that was not cleanly unmounted?
Modifying a forcibly mounted filesystem may not be without risk.
This relates to a problem with the powerpc64 bootloader that we still
haven't fixed. Once your root filesystem gets "dirty", it will remain
dirty forever, even if the OS does a fsck. The solution I came up
with was to drop the MNT_FORCE flag and instead fall back on mounting
the filesystem read-only. That should make the chmod() safe, but it
will fail if you were booting with a dirty root filesystem of course.
I never got an ok for this diff, but I still think it is a good idea,
and it would make your diff safer.
Index: arch/powerpc64/stand/rdboot/disk.c
===================================================================
RCS file: /cvs/src/sys/arch/powerpc64/stand/rdboot/disk.c,v
retrieving revision 1.3
diff -u -p -r1.3 disk.c
--- arch/powerpc64/stand/rdboot/disk.c 29 Aug 2020 11:46:54 -0000 1.3
+++ arch/powerpc64/stand/rdboot/disk.c 23 Sep 2023 11:08:01 -0000
@@ -195,11 +195,13 @@ disk_open(const char *path)
memset(&ffs_args, 0, sizeof(ffs_args));
ffs_args.fspec = devpath;
- if (mount(MOUNT_FFS, "/mnt", MNT_FORCE | MNT_NOATIME,
- &ffs_args) == -1) {
- fprintf(stderr, "failed to mount %s: %s\n", devpath,
- strerror(errno));
- return NULL;
+ if (mount(MOUNT_FFS, "/mnt", MNT_NOATIME, &ffs_args) == -1) {
+ if (mount(MOUNT_FFS, "/mnt", MNT_RDONLY, &ffs_args) == -1) {
+ fprintf(stderr, "failed to mount %s: %s\n", devpath,
+ strerror(errno));
+ return NULL;
+ }
+ fprintf(stderr, "%s: mounted read-only\n", devpath);
}
if (chroot("/mnt") == -1) {
fprintf(stderr, "failed to chroot: %s\n", strerror(errno));
