On Sat, Sep 23, 2023 at 01:11:32PM +0200, Mark Kettenis wrote:
> > 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.

Isn't that already the case with chmo() /etc/random.seed?

Can you explain how that is not a problem in other non-kernel boot loaders
using libsa's ufs2_open() instead of mount(2)?

> 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.

I don't know much about file systems, but not making a dirty root worse
seems sane to me;  better be able to boot without re-sysupgrade detection
and entropy file handling than requiring more fsck(8).

Either way, octeon would need the same treat.

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

Reply via email to