Call for testers - important disklabel(8) diff

Reyk Floeter (reyk@) wrote a mail to the tech mailing list about an important disklabel(8) diff that needs widespread testing.

Please read on for Reyks mail...

List:       openbsd-tech
Subject:    please test: important disklabel diff
From:       Reyk Floeter 
Date:       2008-08-11 16:54:38

hi!

heads up - i attached an important diff for disklabel that should be
tested on all kinds of platforms and partition layouts.

please have a look if disklabel still finds your OpenBSD partition:

# disklabel -E sd0 
# Inside MBR partition 3: type A6 start 40962048 size 193479600
Treating sectors 40962048-234441648 as the OpenBSD portion of the disk.

thanks,
reyk

----- Forwarded message from Reyk Floeter  -----

Date: Mon, 11 Aug 2008 18:48:10 +0200
From: Reyk Floeter 
To: smat@smat.ch
Cc: reyk@cvs.openbsd.org, bugs@cvs.openbsd.org, krw@openbsd.org
Subject: Re: kernel/5905
In-Reply-To: <20080811153105.GA32149@slim.vantronix.net>
User-Agent: Mutt/1.5.12-2006-07-14

hi!

please test this diff and forget about the previous one.

reyk

On Mon, Aug 11, 2008 at 05:31:05PM +0200, Reyk Floeter wrote:
> On Mon, Aug 11, 2008 at 08:18:27AM -0600, reyk@cvs.openbsd.org wrote:
> > Synopsis: installboot problem with OPENBSD_4_4_BASE (i386, amd64)
> > 
> 
> i think the problem is in disklabel because it uses a local static
> variable in the recursive findopenbsd() function where it shouldn't -
> the "mbr" gets overridden by nested calls.
> 
> can you apply the attached diff to src/sbin/disklabel, rewrite the
> disklabel, and check if it is working?  i can't see any problems in
> installboot/biosdev, maybe it is just smashing the disklabel because
> of this static variable.
> 
> please send me the output of "installboot -v" after rewriting the disklabel
> /usr/mdec/installboot -v /mnt/boot /usr/mdec/biosboot
> 
> this was my test:
> ---snip---
> # fdisk sd1                                                 
> Disk: sd1       geometry: 31/255/63 [510976 Sectors]
> Offset: 0       Signature: 0xAA55
>             Starting         Ending         LBA Info:
>  #: id      C   H   S -      C   H   S [       start:        size ]
> -------------------------------------------------------------------------------
>  0: 12      0   1   1 -     10 254  63 [          63:      176652 ] Compaq Diag.
>  1: 07     11   0   1 -     15 254  63 [      176715:       80325 ] HPFS/QNX/AUX
>  2: 0F     16   0   1 -     20 254  63 [      257040:       80325 ] Extended LBA
> *3: A6     21   0   1 -     30 254  63 [      337365:      160650 ] OpenBSD     
> Offset: 257040  Signature: 0xAA55
>             Starting         Ending         LBA Info:
>  #: id      C   H   S -      C   H   S [       start:        size ]
> -------------------------------------------------------------------------------
>  0: 07     16   1   1 -     18 254  63 [      257103:       48132 ] HPFS/QNX/AUX
>  1: 07     19   0   1 -     20 254  63 [      305235:       32130 ] HPFS/QNX/AUX
>  2: 00      0   0   0 -      0   0   0 [           0:           0 ] unused      
>  3: 00      0   0   0 -      0   0   0 [           0:           0 ] unused      
> # disklabel -E sd1                                                             
> # Extended partition 2: type 0F start 257040 size 80325
> # Inside MBR partition 3: type A6 start 337365 size 160650
> Treating sectors 337365-498015 as the OpenBSD portion of the disk.
> You can use the 'b' command to change this.
> 
> Initial label editor (enter '?' for help at any prompt)
> > w
> > q
> No label changes.
> # /usr/mdec/installboot -v /mnt/boot /usr/mdec/biosboot sd1 
> boot: /mnt/boot
> proto: /usr/mdec/biosboot
> device: /dev/rsd1c
> /usr/mdec/biosboot: entry point 0
> proto bootblock size 512
> /mnt/boot is 3 blocks x 16384 bytes
> fs block shift 2; part offset 337365; inode block 24, offset 936
> extended partition 2: type 15 (0x0f) offset 257040 (0x3ec10)
> using MBR partition 3: type 166 (0xa6) offset 337365 (0x525d5)
> ---snap---
> 
> reyk
> 

Index: disklabel.c
===================================================================
RCS file: /cvs/src/sbin/disklabel/disklabel.c,v
retrieving revision 1.135
diff -u -p -r1.135 disklabel.c
--- disklabel.c	10 Aug 2008 13:00:25 -0000	1.135
+++ disklabel.c	11 Aug 2008 16:33:28 -0000
@@ -529,7 +529,8 @@ l_perror(char *s)
 struct dos_partition *
 findopenbsd(int f, off_t mbroff, struct dos_partition **first, int *n)
 {
-	static int mbr[DEV_BSIZE / sizeof(int)];
+	static struct dos_partition res;
+	int mbr[DEV_BSIZE / sizeof(int)];
 	struct dos_partition *dp, *p;
 	u_int16_t signature;
 	u_int32_t start = 0;
@@ -579,9 +580,10 @@ findopenbsd(int f, off_t mbroff, struct 
 			    "type %02X start %u size %u\n",
 			    part, dp[part].dp_typ,
 			    letoh32(dp[part].dp_start), letoh32(dp[part].dp_size));
-			dp[part].dp_start =
-			    htole32((off_t)letoh32(dp[part].dp_start) + mbroff);
-			return (&dp[part]);
+			bcopy(&dp[part], &res, sizeof(struct dos_partition));
+			res.dp_start =
+			    htole32((off_t)letoh32(res.dp_start) + mbroff);
+			return (&res);
 		case DOSPTYP_EXTEND:
 		case DOSPTYP_EXTENDL:
 			fprintf(stderr, "# Extended partition %d: "