9/23/23 14:11, Mark Kettenis пишет:
>> Date: Thu, 21 Sep 2023 22:30:01 +0000
>> From: Klemens Nanni <[email protected]>
>>
>> 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.
Wrt. taking care around filesystem writes and the re-upgrade
prevention, this appraoch seems small enough and reasonable.
Any takers for this and/or my original diff?
>
> 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));
>
