install/56890: sysinst should ignore default label even in DISKLABEL_NO_ONDISK_VERIFY case

[Izumi Tsutsui <tsutsui%ceres.dti.ne.jp@localhost>]

>Number:         56890
>Category:       install
>Synopsis:       sysinst should ignore default label even in DISKLABEL_NO_ONDISK_VERIFY case
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    install-manager
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Thu Jun 16 15:45:03 +0000 2022
>Originator:     Izumi Tsutsui
>Release:        NetBSD 9.99.97 202206111130Z snapshot
>Organization:
>Environment:
System: NetBSD 9.99.97 INSTALL
Architecture: m68k
Machine: x68k, maybe also amiga and some other ports
>Description:
In PR install 54582 https://gnats.netbsd.org/54582 MD
DISKLABEL_NO_ONDISK_VERIFY definitions were introduced to
disable on-disk presence verification of "real" disklabels
for ports that didn't have BSD disklabel but used MD
disk partition information.

However, in that case we should disable "disklabel -r" call
but should still check if the disklabel information returend
by DIOCGDINFO ioctl(2) is the default one, i.e. the target disk
doesn't have vaild label. Otherwise the inner (disklabel) partition
editor mis-detect pertitions per the default one, at least on x68k.

On x68k readdisklabel(9) via DIOCGDINFO returns as following data:
---
# disklabel sd0
# /dev/rsd0:
type: SCSI
disk: FIREBALL1024S
label: default label
flags:
bytes/sector: 512
sectors/track: 32
tracks/cylinder: 64
sectors/cylinder: 2048
cylinders: 1024
total sectors: 2097152
rpm: 3600
interleave: 1
trackskew: 0
cylinderskew: 0
headswitch: 0           # microseconds
track-to-track seek: 0  # microseconds
drivedata: 0

3 partitions:
#        size    offset     fstype [fsize bsize cpg/sgs]
 a:   2097152         0     unused      0     0        # (Cly.      0 -    1023)
 c:   2097152         0     unused      0     0        # (Cly.      0 -    1023)
disklabel: boot block size 0
disklabel: super block size 0
# 
---

then sysinst inner (disklabel) editor shows:

---
We now have your disklabel partitions for sd0 below.  This is your last chance to change
them.

Flags: (I)nstall, (N)ewfs.  Total size: 1024M, free: 32M

    Start (sec)    End (sec)   Size (sec)  FS type Flag Filesystem
   ------------ ------------ ------------ -------- ---- ----------------
a:            0     2097151       2097152   unused
b:            0     2097151       2097151 Whole disk
c:           64     2031679       2031616   4.2BSD IN   /
   ------------ ------------ ------------ -------- ---- ----------------
e: Add a partition
f: Change input units (sectors/cylinders/MB/GB)
g: Edit name of the disk
h: Clone external partition(s)
i: Cancel
x: Partition sizes ok
---

So the default label info should be ignored even on typical installation.

>How-To-Repeat:
See above.

>Fix:
Unfortunately it seems there is no MI way to detect whether the MD
readdisklabel(9) via DIOCGDINFO ioctl(2) returns the on-disk label
or the "default" one in case that the disk has no valid label.

For workaround, we can check possilbe default label values per
MD readdisklabel(9) implementation and perform existing "faked"
label code.

---

Index: usr.sbin/sysinst/disklabel.c
===================================================================
RCS file: /cvsroot/src/usr.sbin/sysinst/disklabel.c,v
retrieving revision 1.44
diff -u -p -d -r1.44 disklabel.c
--- usr.sbin/sysinst/disklabel.c	8 Aug 2021 21:50:10 -0000	1.44
+++ usr.sbin/sysinst/disklabel.c	15 Jun 2022 16:31:13 -0000
@@ -198,16 +198,7 @@ disklabel_parts_read(const char *disk, d
 	int fd;
 	char diskpath[MAXPATHLEN];
 	uint flags;
-#ifndef DISKLABEL_NO_ONDISK_VERIFY
-	bool have_raw_label = false;
-
-	/*
-	 * Verify we really have a disklabel.
-	 */
-	if (run_program(RUN_SILENT | RUN_ERROR_OK,
-	    "disklabel -r %s", disk) == 0)
-		have_raw_label = true;
-#endif
+	bool have_ondisk_label;
 
 	/* read partitions */
 
@@ -304,8 +295,52 @@ disklabel_parts_read(const char *disk, d
 	}
 	close(fd);
 
+	/*
+	 * Verify we really have a disklabel on the target disk.
+	 */
 #ifndef DISKLABEL_NO_ONDISK_VERIFY
-	if (!have_raw_label) {
+	have_ondisk_label = false;
+	if (run_program(RUN_SILENT | RUN_ERROR_OK,
+	    "disklabel -r %s", disk) == 0)
+		have_ondisk_label = true;
+#else
+	/*
+	 * disklabel(8) -r checks a native disklabel at LABELOFFSET sector,
+	 * but several ports don't have a native label and use emulated one
+	 * translated from port specific MD disk partition information.
+	 * Unfortunately, there is no MI way to check whether the disk has
+	 * a native BSD disklabel by readdisklabel(9) via DIOCGDINFO.
+	 * So check if returned label looks defaults set by readdisklabel(9).
+	 *
+	 * XXX these should be checked in MD hook functions.
+	 */
+
+	/* assume we have a label by default */
+	have_ondisk_label = true;
+
+	/* check default values on amiga */
+	if (parts->l.d_npartitions == RAW_PART + 1 &&
+	    parts->l.d_partitions[RAW_PART].p_size == 0x1fffffff &&
+	    parts->l.d_partitions[0].p_size ==
+	     parts->l.d_partitions[RAW_PART].p_size &&
+	    parts->l.d_partitions[0].p_offset == 0 &&
+	    parts->l.d_partitions[0].p_fstype == FS_BSDFFS) {
+		have_ondisk_label = false;
+	}
+
+	/* XXX maybe we should also check mac68k with Apple Partition Map */
+
+	/* check default values on x68k */
+	if (parts->l.d_npartitions == RAW_PART + 1 &&
+	    parts->l.d_partitions[0].p_size ==
+	     parts->l.d_partitions[RAW_PART].p_size &&
+	    parts->l.d_partitions[0].p_fstype == FS_UNUSED &&
+	    parts->l.d_bbsize == 0 && parts->l.d_sbsize == 0) {
+		have_ondisk_label = false;
+	}
+#endif
+
+	if (!have_ondisk_label) {
 		bool found_real_part = false;
 
 		if (parts->l.d_npartitions <= RAW_PART ||
@@ -338,7 +373,6 @@ no_valid_label:
 			return NULL;
 		}
 	}
-#endif
 
 	return &parts->dp;
 }

---

As noted in the above patch, MD checks should be implemented
in port speicifc md.c, but currently disklabel.c has no MD hooks.

Another possible workaround is to modify all MD readdisklabel(9)
(at least on amiga) to set non-zero "sbsize" and "bbsize" values
if the target disk has valid MD native label converted to the
BSD disklabel, and make sysinst check sbsize and bbsize, as
disklabel(8) already does.

---
Izumi Tsutsui