git: 4445d039583a - main - stand: userboot: improve userdisk error handling

From: Roman Bogorodskiy <novel_at_FreeBSD.org>
Date: Tue, 06 Oct 2026 16:59:05 UTC
The branch main has been updated by novel:

URL: https://cgit.FreeBSD.org/src/commit/?id=4445d039583a0a47fc335e0d34401581bcbc104e

commit 4445d039583a0a47fc335e0d34401581bcbc104e
Author:     Roman Bogorodskiy <novel@FreeBSD.org>
AuthorDate: 2026-08-28 16:43:26 +0000
Commit:     Roman Bogorodskiy <novel@FreeBSD.org>
CommitDate: 2026-10-06 16:51:40 +0000

    stand: userboot: improve userdisk error handling
    
    Currently, userdisk_init() iterates through disks and
    checks whether DIOCGSECTORSIZE and DIOCGMEDIASIZE ioctls
    are available for the device. If either ioctl fails, userdisk_init()
    returns ENXIO with ud_info only partially initialized.
    devinit() records the failure, but loader_main() ignores
    devinit()'s return value, so later code may access an
    uninitialized entry.
    
    In certain cases this code can still boot from a disk even if
    userdisk_init() returns ENXIO, for example, when the first of
    two disks is valid and the second errors out.
    
    To address this issue while not breaking the current behavior, do
    the following:
    
     - Zero-initialize the userdisk_info array and use a zero media size
       to mark disks that do not support the ioctls mentioned above or
       report an empty medium.
     - Update userdisk_print() not to print unusable disks.
     - Update userdisk_open() to return EIO for unusable disks.
     - Update extract_currdev() to pick the first usable disk
       instead of the first disk. Introduce userboot_disk_firstunit()
       for that.
    
    Additionally, reset counters on ud_info allocation failure to prevent
    accessing unallocated data.
    
    Reviewed by:    jhb (previous version), markj
    MFC after:      2 weeks
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D59262
---
 stand/userboot/userboot/libuserboot.h   |  1 +
 stand/userboot/userboot/main.c          |  9 ++++---
 stand/userboot/userboot/userboot_disk.c | 45 +++++++++++++++++++++++----------
 3 files changed, 39 insertions(+), 16 deletions(-)

diff --git a/stand/userboot/userboot/libuserboot.h b/stand/userboot/userboot/libuserboot.h
index e1584ec29412..0844e62684f6 100644
--- a/stand/userboot/userboot/libuserboot.h
+++ b/stand/userboot/userboot/libuserboot.h
@@ -39,6 +39,7 @@ typedef unsigned long physaddr_t;
 /* exported devices */
 extern struct devsw userboot_disk;
 extern int userboot_disk_maxunit;
+int userboot_disk_firstunit(void);
 extern struct devsw host_dev;
 
 /* access to host filesystem */
diff --git a/stand/userboot/userboot/main.c b/stand/userboot/userboot/main.c
index d1f231a12e97..985933bf51e9 100644
--- a/stand/userboot/userboot/main.c
+++ b/stand/userboot/userboot/main.c
@@ -237,9 +237,13 @@ extract_currdev(void)
 {
 	struct disk_devdesc dev;
 	struct devdesc *dd;
+	int diskunit;
 #if defined(USERBOOT_ZFS_SUPPORT)
 	struct zfs_devdesc zdev;
+#endif
 
+	diskunit = userboot_disk_firstunit();
+#if defined(USERBOOT_ZFS_SUPPORT)
 	if (userboot_zfs_found) {
 	
 		/* Leave the pool/root guid's unassigned */
@@ -250,10 +254,9 @@ extract_currdev(void)
 		dd = &zdev.dd;
 	} else
 #endif
-
-	if (userboot_disk_maxunit > 0) {
+	if (diskunit >= 0) {
 		dev.dd.d_dev = &userboot_disk;
-		dev.dd.d_unit = 0;
+		dev.dd.d_unit = diskunit;
 		dev.d_slice = D_SLICEWILD;
 		dev.d_partition = D_PARTWILD;
 		/*
diff --git a/stand/userboot/userboot/userboot_disk.c b/stand/userboot/userboot/userboot_disk.c
index 9adcad4f00ac..78377c582225 100644
--- a/stand/userboot/userboot/userboot_disk.c
+++ b/stand/userboot/userboot/userboot_disk.c
@@ -82,25 +82,29 @@ userdisk_init(void)
 	off_t mediasize;
 	u_int sectorsize;
 	int i;
+	int userdisk_usable = 0;
 
 	userdisk_maxunit = userboot_disk_maxunit;
 	if (userdisk_maxunit > 0) {
-		ud_info = malloc(sizeof(*ud_info) * userdisk_maxunit);
-		if (ud_info == NULL)
+		ud_info = calloc(userdisk_maxunit, sizeof(*ud_info));
+		if (ud_info == NULL) {
+			userdisk_maxunit = 0;
+			userboot_disk_maxunit = 0;
 			return (ENOMEM);
+		}
 		for (i = 0; i < userdisk_maxunit; i++) {
 			if (CALLBACK(diskioctl, i, DIOCGSECTORSIZE,
-			    &sectorsize) != 0 || CALLBACK(diskioctl, i,
-			    DIOCGMEDIASIZE, &mediasize) != 0)
-				return (ENXIO);
-			ud_info[i].mediasize = mediasize;
-			ud_info[i].sectorsize = sectorsize;
-			ud_info[i].ud_open = 0;
-			ud_info[i].ud_bcache = NULL;
+			    &sectorsize) == 0 && CALLBACK(diskioctl, i,
+			    DIOCGMEDIASIZE, &mediasize) == 0 && mediasize > 0) {
+				ud_info[i].mediasize = mediasize;
+				ud_info[i].sectorsize = sectorsize;
+				userdisk_usable++;
+			}
 		}
 	}
-	bcache_add_dev(userdisk_maxunit);
-	return(0);
+	bcache_add_dev(userdisk_usable);
+
+	return (0);
 }
 
 static void
@@ -111,6 +115,18 @@ userdisk_cleanup(void)
 		free(ud_info);
 }
 
+int
+userboot_disk_firstunit(void)
+{
+	int i;
+
+	for (i = 0; i < userdisk_maxunit; i++) {
+		if (ud_info[i].mediasize != 0)
+			return (i);
+	}
+	return (-1);
+}
+
 /*
  * Print information about disks
  */
@@ -121,7 +137,7 @@ userdisk_print(int verbose)
 	char line[80];
 	int i, ret = 0;
 
-	if (userdisk_maxunit == 0)
+	if (userboot_disk_firstunit() < 0)
 		return (0);
 
 	printf("%s devices:", userboot_disk.dv_name);
@@ -129,6 +145,8 @@ userdisk_print(int verbose)
 		return (ret);
 
 	for (i = 0; i < userdisk_maxunit; i++) {
+		if (ud_info[i].mediasize == 0)
+			continue;
 		snprintf(line, sizeof(line),
 		    "    disk%d:   Guest drive image\n", i);
 		ret = pager_output(line);
@@ -163,7 +181,8 @@ userdisk_open(struct open_file *f, ...)
 	dev = va_arg(ap, struct disk_devdesc *);
 	va_end(ap);
 
-	if (dev->dd.d_unit < 0 || dev->dd.d_unit >= userdisk_maxunit)
+	if (dev->dd.d_unit < 0 || dev->dd.d_unit >= userdisk_maxunit ||
+	    ud_info[dev->dd.d_unit].mediasize == 0)
 		return (EIO);
 	ud_info[dev->dd.d_unit].ud_open++;
 	if (ud_info[dev->dd.d_unit].ud_bcache == NULL)