[PATCH v1 0/2] ppc64le: fix problems with signed boot media

[PATCH v1 0/2] ppc64le: fix problems with signed boot media

[Robbie Harwood]

Fix three issues uncovered while trying to use the same core.elf on both
installed systems and boot media.  CCing POWER folks I have working
emails for in case they have a better way to do any of this.

Be well,
--Robbie

Robbie Harwood (2):
  Skip diskfilter on powerpc CDs
  Fix prefix and root on ppc64le CDs

 grub-core/disk/diskfilter.c |  5 +++++
 grub-core/kern/main.c       | 17 +++++++++++++++++
 2 files changed, 22 insertions(+)

-- 
2.39.0

[PATCH v1 1/2] Skip diskfilter on powerpc CDs

[Robbie Harwood]

Prior to this change, on ppc64le with part_msdos and the mdraid* modules
enabled, we see:

    disk/diskfilter.c:191: scanning ieee1275/cdrom
    kern/disk.c:196: Opening `ieee1275/cdrom'...
    disk/ieee1275/ofdisk.c:477: Opening `cdrom'.
    disk/ieee1275/ofdisk.c:502: MAX_RETRIES set to 20
    kern/disk.c:288: Opening `ieee1275/cdrom' succeeded.
    disk/diskfilter.c:136: Scanning for DISKFILTER devices on disk ieee1275/cdrom
    partmap/msdos.c:184: partition 0: flag 0x80, type 0x96, start 0x0, len
    0x6a5d70
    disk/diskfilter.c:136: Scanning for DISKFILTER devices on disk ieee1275/cdrom
    SCSI-DISK: Access beyond end of device !
    SCSI-DISK: Access beyond end of device !
    SCSI-DISK: Access beyond end of device !
    SCSI-DISK: Access beyond end of device !
    SCSI-DISK: Access beyond end of device !
    disk/ieee1275/ofdisk.c:578: MAX_RETRIES set to 20

These latter two lines repeat many times, eventually ending in:

    kern/disk.c:388: ieee1275/cdrom read failed
    error: ../../grub-core/disk/ieee1275/ofdisk.c:608:failure reading sector
    0x1a9720 from `ieee1275/cdrom'.

and the system drops to a "grub>" prompt.

The firmware and the iso image appear to diagree on the blocksize (512
vs. 2048), and the diskfilter RAID probing is too much for it.  We won't
be seeing RAID on bootable CDs, so just turn it off.

Signed-off-by: Robbie Harwood <rharwood@redhat.com>
---
 grub-core/disk/diskfilter.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c
index 1c568927b8..35f45cb95e 100644
--- a/grub-core/disk/diskfilter.c
+++ b/grub-core/disk/diskfilter.c
@@ -133,6 +133,11 @@ scan_disk_partition_iter (grub_disk_t disk, grub_partition_t p, void *data)
   struct grub_diskfilter_pv_id id;
   grub_diskfilter_t diskfilter;
 
+#ifdef __powerpc__
+  if (!grub_strcmp (name, "ieee1275/cdrom"))
+    return 0;
+#endif
+
   grub_dprintf ("diskfilter", "Scanning for DISKFILTER devices on disk %s\n",
 		name);
 #ifdef GRUB_UTIL
-- 
2.39.0

[PATCH v1 2/2] Fix prefix and root on ppc64le CDs

[Robbie Harwood]

CHRP requires grub at /boot/grub.  On CDs, this means any other prefix
will be invalid.  In particular, a distro grub might prefer /grub2 - but
prefix is hardcoded into the signed core.elf, and will always be wrong
here.

Also work around a conflict between OF naming and that used by
part_msdos: on CDs, we always want the raw device.  This fixes an issue
where grub would successfully load the menu from an image, but then
produce the error:

    error: ../../grub-core/net/net.c:1552:disk `ieee1275/cdrom,0' not found.

and fail to boot further.

Signed-off-by: Robbie Harwood <rharwood@redhat.com>
---
 grub-core/kern/main.c | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/grub-core/kern/main.c b/grub-core/kern/main.c
index 731c07c290..0a24fe38e4 100644
--- a/grub-core/kern/main.c
+++ b/grub-core/kern/main.c
@@ -216,6 +216,23 @@ grub_set_prefix_and_root (void)
 	  grub_free (prefix_set);
 	}
       grub_env_set ("root", device);
+
+#ifdef __powerpc__
+      /* When booting from a CD, if part_msdos is enabled, grub will detect
+         and create access points starting at (ieee1275/cdrom,msdos1).
+         However, the device detection and OF name transformation will produce
+         a device named (ieee1275/cdrom,0) - i.e., missing the msdos and also
+         differently indexed.  Furthermore, CHRP mandates boot/grub as prefix,
+         but our signed images are built with /grub2 to reflect installed
+         systems.  Just ignore both messes.
+       */
+      if (!grub_strncmp (device[0] == '(' ? device + 1 : device,
+                         "ieee1275/cdrom", grub_strlen ("ieee1275/cdrom")))
+        {
+          grub_env_set ("prefix", "/boot/grub");
+          grub_env_set ("root", "ieee1275/cdrom");
+        }
+#endif
     }
 
   grub_free (device);
-- 
2.39.0

Re: [PATCH v1 2/2] Fix prefix and root on ppc64le CDs

[Vladimir 'phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 2528 bytes --]

The ,0 suffix sounds like a bug in a detection
As for the prefix, can‘t we solve it by setting prefix properly.
If there is really no better solution (doubtful) then it needs to be
restricted to chrp platforms and even there, probably only a subset
On Fri, 13 Jan 2023 at 23:27, Robbie Harwood <rharwood@redhat.com> wrote:

> CHRP requires grub at /boot/grub.  On CDs, this means any other prefix
> will be invalid.  In particular, a distro grub might prefer /grub2 - but
> prefix is hardcoded into the signed core.elf, and will always be wrong
> here.
>
> Also work around a conflict between OF naming and that used by
> part_msdos: on CDs, we always want the raw device.  This fixes an issue
> where grub would successfully load the menu from an image, but then
> produce the error:
>
>     error: ../../grub-core/net/net.c:1552:disk `ieee1275/cdrom,0' not
> found.
>
> and fail to boot further.
>
> Signed-off-by: Robbie Harwood <rharwood@redhat.com>
> ---
>  grub-core/kern/main.c | 17 +++++++++++++++++
>  1 file changed, 17 insertions(+)
>
> diff --git a/grub-core/kern/main.c b/grub-core/kern/main.c
> index 731c07c290..0a24fe38e4 100644
> --- a/grub-core/kern/main.c
> +++ b/grub-core/kern/main.c
> @@ -216,6 +216,23 @@ grub_set_prefix_and_root (void)
>           grub_free (prefix_set);
>         }
>        grub_env_set ("root", device);
> +
> +#ifdef __powerpc__
> +      /* When booting from a CD, if part_msdos is enabled, grub will
> detect
> +         and create access points starting at (ieee1275/cdrom,msdos1).
> +         However, the device detection and OF name transformation will
> produce
> +         a device named (ieee1275/cdrom,0) - i.e., missing the msdos and
> also
> +         differently indexed.  Furthermore, CHRP mandates boot/grub as
> prefix,
> +         but our signed images are built with /grub2 to reflect installed
> +         systems.  Just ignore both messes.
> +       */
> +      if (!grub_strncmp (device[0] == '(' ? device + 1 : device,
> +                         "ieee1275/cdrom", grub_strlen
> ("ieee1275/cdrom")))
> +        {
> +          grub_env_set ("prefix", "/boot/grub");
> +          grub_env_set ("root", "ieee1275/cdrom");
> +        }
> +#endif
>      }
>
>    grub_free (device);
> --
> 2.39.0
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel
>
-- 
Regards
Vladimir 'phcoder' Serbinenko

[-- Attachment #2: Type: text/html, Size: 3499 bytes --]

Re: [PATCH v1 2/2] Fix prefix and root on ppc64le CDs

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 374 bytes --]

"Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com> writes:

> The ,0 suffix sounds like a bug in a detection
> As for the prefix, can‘t we solve it by setting prefix properly.
> If there is really no better solution (doubtful) then it needs to be
> restricted to chrp platforms and even there, probably only a subset

Isn't that what I did?

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]

Re: [PATCH v1 1/2] Skip diskfilter on powerpc CDs

[Vladimir 'phcoder' Serbinenko]

This is not properly restricted to just raid. Only mdraid 1.1 is
affected. The condition should probably be along the lines "if msdos
is embedded in a device of unknown size, then skip mdraid 1.1 probing"

On Fri, Jan 13, 2023 at 9:27 PM Robbie Harwood <rharwood@redhat.com> wrote:
>
> Prior to this change, on ppc64le with part_msdos and the mdraid* modules
> enabled, we see:
>
>     disk/diskfilter.c:191: scanning ieee1275/cdrom
>     kern/disk.c:196: Opening `ieee1275/cdrom'...
>     disk/ieee1275/ofdisk.c:477: Opening `cdrom'.
>     disk/ieee1275/ofdisk.c:502: MAX_RETRIES set to 20
>     kern/disk.c:288: Opening `ieee1275/cdrom' succeeded.
>     disk/diskfilter.c:136: Scanning for DISKFILTER devices on disk ieee1275/cdrom
>     partmap/msdos.c:184: partition 0: flag 0x80, type 0x96, start 0x0, len
>     0x6a5d70
>     disk/diskfilter.c:136: Scanning for DISKFILTER devices on disk ieee1275/cdrom
>     SCSI-DISK: Access beyond end of device !
>     SCSI-DISK: Access beyond end of device !
>     SCSI-DISK: Access beyond end of device !
>     SCSI-DISK: Access beyond end of device !
>     SCSI-DISK: Access beyond end of device !
>     disk/ieee1275/ofdisk.c:578: MAX_RETRIES set to 20
>
> These latter two lines repeat many times, eventually ending in:
>
>     kern/disk.c:388: ieee1275/cdrom read failed
>     error: ../../grub-core/disk/ieee1275/ofdisk.c:608:failure reading sector
>     0x1a9720 from `ieee1275/cdrom'.
>
> and the system drops to a "grub>" prompt.
>
> The firmware and the iso image appear to diagree on the blocksize (512
> vs. 2048), and the diskfilter RAID probing is too much for it.  We won't
> be seeing RAID on bootable CDs, so just turn it off.
>
> Signed-off-by: Robbie Harwood <rharwood@redhat.com>
> ---
>  grub-core/disk/diskfilter.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/grub-core/disk/diskfilter.c b/grub-core/disk/diskfilter.c
> index 1c568927b8..35f45cb95e 100644
> --- a/grub-core/disk/diskfilter.c
> +++ b/grub-core/disk/diskfilter.c
> @@ -133,6 +133,11 @@ scan_disk_partition_iter (grub_disk_t disk, grub_partition_t p, void *data)
>    struct grub_diskfilter_pv_id id;
>    grub_diskfilter_t diskfilter;
>
> +#ifdef __powerpc__
> +  if (!grub_strcmp (name, "ieee1275/cdrom"))
> +    return 0;
> +#endif
> +
>    grub_dprintf ("diskfilter", "Scanning for DISKFILTER devices on disk %s\n",
>                 name);
>  #ifdef GRUB_UTIL
> --
> 2.39.0
>
>
> _______________________________________________
> Grub-devel mailing list
> Grub-devel@gnu.org
> https://lists.gnu.org/mailman/listinfo/grub-devel



-- 
Regards
Vladimir 'phcoder' Serbinenko

Re: [PATCH v1 1/2] Skip diskfilter on powerpc CDs

[Robbie Harwood]

[-- Attachment #1: Type: text/plain, Size: 604 bytes --]

"Vladimir 'phcoder' Serbinenko" <phcoder@gmail.com> writes:

> This is not properly restricted to just raid. Only mdraid 1.1 is
> affected. The condition should probably be along the lines "if msdos
> is embedded in a device of unknown size, then skip mdraid 1.1 probing"

My testing suggested it was both mdraid1x and mdraid09 - that is, the
presence of either module was sufficient to trigger the issue.  While
I'm not an mdraid expert, I don't see other users of the diskfilter
interface in the tree.

If you have an alternate patch, I am happy to test whether it solves the
issue.

Be well,
--Robbie

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 861 bytes --]