Index | Thread | Search

From:
Jan Klemkow <jan@openbsd.org>
Subject:
Re: softraid: Correct LBA start offset type
To:
Thomas Habets <thomas@habets.se>
Cc:
tech@openbsd.org
Date:
Mon, 13 Jul 2026 23:39:12 +0200

Download raw body.

Thread
Hi Thomas,

On Mon, Jul 13, 2026 at 09:51:12AM -0500, Thomas Habets wrote:
> In softraid, unlike in efidev.c, `findopenbsd_gpt()` returns `u_int64_t`. But
> it's then assigned to an `u_int`. While the `-1` check should still work,
> offsets beyond 2TB probably fail.
> 
> For i386 the change is a no-op since `u_int` is `u_int32_t`, but since the
> assignment of `start` comes from an `u_int32_t`, it seems more consistent to me.

Thanks for your report.  Btw, your diff was broken and not applicable, because
you converted all tabs to spaces.  Anyway...

Yes, start have to be big enough to store the return values of
sr_getdisklabel().  I'm not sure if you (uint64_t)-1 cast is still needed.
Or, just if (start == -1) could also be sufficient.

But, instead of assigning -1 to an unsigned data type.  We could also use the
UINT64_MAX constant to indicate the error condition.  At least this looks more
straight forward to me, thought.

ok?

bye,
Jan

Index: arch/amd64/stand/libsa/softraid_amd64.c
===================================================================
RCS file: /cvs/src/sys/arch/amd64/stand/libsa/softraid_amd64.c,v
diff -u -p -r1.11 softraid_amd64.c
--- arch/amd64/stand/libsa/softraid_amd64.c	18 Nov 2025 15:24:09 -0000	1.11
+++ arch/amd64/stand/libsa/softraid_amd64.c	13 Jul 2026 21:00:19 -0000
@@ -454,12 +454,12 @@ findopenbsd_gpt(struct sr_boot_volume *b
 
 	if (bv->sbv_secsize > 4096) {
 		*err = "disk sector > 4096 bytes\n";
-		return (-1);
+		return (UINT64_MAX);
 	}
 	buf = alloc(bv->sbv_secsize);
 	if (buf == NULL) {
 		*err = "out of memory\n";
-		return (-1);
+		return (UINT64_MAX);
 	}
 	bzero(buf, bv->sbv_secsize);
 
@@ -473,20 +473,20 @@ findopenbsd_gpt(struct sr_boot_volume *b
 	if (letoh64(gh.gh_sig) != GPTSIGNATURE) {
 		*err = "bad GPT signature\n";
 		free(buf, bv->sbv_secsize);
-		return (-1);
+		return (UINT64_MAX);
 	}
 
 	if (letoh32(gh.gh_rev) != GPTREVISION) {
 		*err = "bad GPT revision\n";
 		free(buf, bv->sbv_secsize);
-		return (-1);
+		return (UINT64_MAX);
 	}
 
 	ghsize = letoh32(gh.gh_size);
 	if (ghsize < GPTMINHDRSIZE || ghsize > sizeof(struct gpt_header)) {
 		*err = "bad GPT header size\n";
 		free(buf, bv->sbv_secsize);
-		return (-1);
+		return (UINT64_MAX);
 	}
 
 	/* Check checksum */
@@ -497,7 +497,7 @@ findopenbsd_gpt(struct sr_boot_volume *b
 	if (letoh32(orig_csum) != new_csum) {
 		*err = "bad GPT header checksum\n";
 		free(buf, bv->sbv_secsize);
-		return (-1);
+		return (UINT64_MAX);
 	}
 
 	lba = letoh64(gh.gh_part_lba);
@@ -529,12 +529,12 @@ findopenbsd_gpt(struct sr_boot_volume *b
 
 	if (new_csum != letoh32(gh.gh_part_csum)) {
 		*err = "bad GPT entries checksum\n";
-		return (-1);
+		return (UINT64_MAX);
 	}
 	if (found)
 		return (letoh64(gp.gp_lba_start));
 
-	return (-1);
+	return (UINT64_MAX);
 }
 
 const char *
@@ -543,7 +543,7 @@ sr_getdisklabel(struct sr_boot_volume *b
 	struct dos_partition *dp;
 	struct dos_mbr mbr;
 	const char *err = NULL;
-	u_int start = 0;
+	uint64_t start = 0;
 	char buf[DEV_BSIZE];
 	int i;
 
@@ -553,7 +553,7 @@ sr_getdisklabel(struct sr_boot_volume *b
 	if (gpt_chk_mbr(mbr.dmbr_parts, bv->sbv_size /
 		    (bv->sbv_secsize / DEV_BSIZE)) == 0) {
 		start = findopenbsd_gpt(bv, &err);
-		if (start == (u_int)-1) {
+		if (start == UINT64_MAX) {
 			if (err != NULL)
 				return (err);
 			return "no OpenBSD partition\n";
Index: arch/arm64/stand/efiboot/softraid_arm64.c
===================================================================
RCS file: /cvs/src/sys/arch/arm64/stand/efiboot/softraid_arm64.c,v
diff -u -p -r1.7 softraid_arm64.c
--- arch/arm64/stand/efiboot/softraid_arm64.c	18 Nov 2025 15:24:09 -0000	1.7
+++ arch/arm64/stand/efiboot/softraid_arm64.c	13 Jul 2026 21:05:00 -0000
@@ -446,12 +446,12 @@ findopenbsd_gpt(struct sr_boot_volume *b
 
 	if (bv->sbv_secsize > 4096) {
 		*err = "disk sector > 4096 bytes\n";
-		return (-1);
+		return (UINT64_MAX);
 	}
 	buf = alloc(bv->sbv_secsize);
 	if (buf == NULL) {
 		*err = "out of memory\n";
-		return (-1);
+		return (UINT64_MAX);
 	}
 	bzero(buf, bv->sbv_secsize);
 
@@ -465,20 +465,20 @@ findopenbsd_gpt(struct sr_boot_volume *b
 	if (letoh64(gh.gh_sig) != GPTSIGNATURE) {
 		*err = "bad GPT signature\n";
 		free(buf, bv->sbv_secsize);
-		return (-1);
+		return (UINT64_MAX);
 	}
 
 	if (letoh32(gh.gh_rev) != GPTREVISION) {
 		*err = "bad GPT revision\n";
 		free(buf, bv->sbv_secsize);
-		return (-1);
+		return (UINT64_MAX);
 	}
 
 	ghsize = letoh32(gh.gh_size);
 	if (ghsize < GPTMINHDRSIZE || ghsize > sizeof(struct gpt_header)) {
 		*err = "bad GPT header size\n";
 		free(buf, bv->sbv_secsize);
-		return (-1);
+		return (UINT64_MAX);
 	}
 
 	/* Check checksum */
@@ -489,7 +489,7 @@ findopenbsd_gpt(struct sr_boot_volume *b
 	if (letoh32(orig_csum) != new_csum) {
 		*err = "bad GPT header checksum\n";
 		free(buf, bv->sbv_secsize);
-		return (-1);
+		return (UINT64_MAX);
 	}
 
 	lba = letoh64(gh.gh_part_lba);
@@ -521,12 +521,12 @@ findopenbsd_gpt(struct sr_boot_volume *b
 
 	if (new_csum != letoh32(gh.gh_part_csum)) {
 		*err = "bad GPT entries checksum\n";
-		return (-1);
+		return (UINT64_MAX);
 	}
 	if (found)
 		return (letoh64(gp.gp_lba_start));
 
-	return (-1);
+	return (UINT64_MAX);
 }
 
 const char *
@@ -535,7 +535,7 @@ sr_getdisklabel(struct sr_boot_volume *b
 	struct dos_partition *dp;
 	struct dos_mbr mbr;
 	const char *err = NULL;
-	u_int start = 0;
+	uint64_t start = 0;
 	char buf[DEV_BSIZE];
 	int i;
 
@@ -545,7 +545,7 @@ sr_getdisklabel(struct sr_boot_volume *b
 	if (gpt_chk_mbr(mbr.dmbr_parts, bv->sbv_size /
 		    (bv->sbv_secsize / DEV_BSIZE)) == 0) {
 		start = findopenbsd_gpt(bv, &err);
-		if (start == (u_int)-1) {
+		if (start == UINT64_MAX) {
 			if (err != NULL)
 				return (err);
 			return "no OpenBSD partition\n";
Index: arch/riscv64/stand/efiboot/softraid_riscv64.c
===================================================================
RCS file: /cvs/src/sys/arch/riscv64/stand/efiboot/softraid_riscv64.c,v
diff -u -p -r1.6 softraid_riscv64.c
--- arch/riscv64/stand/efiboot/softraid_riscv64.c	18 Nov 2025 15:24:09 -0000	1.6
+++ arch/riscv64/stand/efiboot/softraid_riscv64.c	13 Jul 2026 21:09:06 -0000
@@ -446,12 +446,12 @@ findopenbsd_gpt(struct sr_boot_volume *b
 
 	if (bv->sbv_secsize > 4096) {
 		*err = "disk sector > 4096 bytes\n";
-		return (-1);
+		return (UINT64_MAX);
 	}
 	buf = alloc(bv->sbv_secsize);
 	if (buf == NULL) {
 		*err = "out of memory\n";
-		return (-1);
+		return (UINT64_MAX);
 	}
 	bzero(buf, bv->sbv_secsize);
 
@@ -465,20 +465,20 @@ findopenbsd_gpt(struct sr_boot_volume *b
 	if (letoh64(gh.gh_sig) != GPTSIGNATURE) {
 		*err = "bad GPT signature\n";
 		free(buf, bv->sbv_secsize);
-		return (-1);
+		return (UINT64_MAX);
 	}
 
 	if (letoh32(gh.gh_rev) != GPTREVISION) {
 		*err = "bad GPT revision\n";
 		free(buf, bv->sbv_secsize);
-		return (-1);
+		return (UINT64_MAX);
 	}
 
 	ghsize = letoh32(gh.gh_size);
 	if (ghsize < GPTMINHDRSIZE || ghsize > sizeof(struct gpt_header)) {
 		*err = "bad GPT header size\n";
 		free(buf, bv->sbv_secsize);
-		return (-1);
+		return (UINT64_MAX);
 	}
 
 	/* Check checksum */
@@ -489,7 +489,7 @@ findopenbsd_gpt(struct sr_boot_volume *b
 	if (letoh32(orig_csum) != new_csum) {
 		*err = "bad GPT header checksum\n";
 		free(buf, bv->sbv_secsize);
-		return (-1);
+		return (UINT64_MAX);
 	}
 
 	lba = letoh64(gh.gh_part_lba);
@@ -521,12 +521,12 @@ findopenbsd_gpt(struct sr_boot_volume *b
 
 	if (new_csum != letoh32(gh.gh_part_csum)) {
 		*err = "bad GPT entries checksum\n";
-		return (-1);
+		return (UINT64_MAX);
 	}
 	if (found)
 		return (letoh64(gp.gp_lba_start));
 
-	return (-1);
+	return (UINT64_MAX);
 }
 
 const char *
@@ -535,7 +535,7 @@ sr_getdisklabel(struct sr_boot_volume *b
 	struct dos_partition *dp;
 	struct dos_mbr mbr;
 	const char *err = NULL;
-	u_int start = 0;
+	uint64_t start = 0;
 	char buf[DEV_BSIZE];
 	int i;
 
@@ -545,7 +545,7 @@ sr_getdisklabel(struct sr_boot_volume *b
 	if (gpt_chk_mbr(mbr.dmbr_parts, bv->sbv_size /
 		    (bv->sbv_secsize / DEV_BSIZE)) == 0) {
 		start = findopenbsd_gpt(bv, &err);
-		if (start == (u_int)-1) {
+		if (start == UINT64_MAX) {
 			if (err != NULL)
 				return (err);
 			return "no OpenBSD partition\n";