git: 055b41056ef7 - main - newbus: Limit units to [0, INT_MAX)

From: Warner Losh <imp_at_FreeBSD.org>
Date: Fri, 01 Nov 2024 17:55:49 UTC
The branch main has been updated by imp:

URL: https://cgit.FreeBSD.org/src/commit/?id=055b41056ef7a54d0a75ba5c9049fc0bd34a8b26

commit 055b41056ef7a54d0a75ba5c9049fc0bd34a8b26
Author:     Warner Losh <imp@FreeBSD.org>
AuthorDate: 2024-10-31 22:50:54 +0000
Commit:     Warner Losh <imp@FreeBSD.org>
CommitDate: 2024-11-01 17:56:20 +0000

    newbus: Limit units to [0, INT_MAX)
    
    Limit the number of units a newbus device can have to be a positive
    number. Reserve and reject the unit INT_MAX so that we can set maxunit
    to INT_MAX without ill effect and so the normal signed int math
    works. Add sanity checks to make sure we don't get negative unit numbers
    from bus routines that can set the unit. Remove now-redundant check for
    unit >=0 since it must be after an earlier check.
    
    This should be largely a nop, since we'll likely run out of memory
    before we have 2^31 devices. Also, finding unit number is O(n^2) since
    we do linear searches for the next unit number, which even on very fast
    machines will grind to a halt well before we reach this limit...
    
    Add note to device_find_free_unit that says it can return INT_MAX when
    all the unit numbers are in use. The one user in the tree
    (ata_pci_attach) will then add a child with this unit and it will fail
    and that failure will be handled properly. Hardware limitations, though
    mean there will never be more than tens of units, let alone billions.
    
    Update docs to document that EINVAL can be returned for bogus unit
    numbers, or when we run out.
    
    Sponsored-by:           Netflix
    Reviewed-by:            jhb
    Differential-Revision:  https://reviews.freebsd.org/D47359
    Co-Authored-by: Elliott Mitchell <ehem+freebsd@m5p.com>
---
 sys/kern/subr_bus.c | 23 +++++++++++++++++++----
 1 file changed, 19 insertions(+), 4 deletions(-)

diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c
index cd5f97cc72fe..c1f75dd30126 100644
--- a/sys/kern/subr_bus.c
+++ b/sys/kern/subr_bus.c
@@ -1121,7 +1121,8 @@ devclass_get_maxunit(devclass_t dc)
  * @brief Find a free unit number in a devclass
  *
  * This function searches for the first unused unit number greater
- * that or equal to @p unit.
+ * that or equal to @p unit. Note: This can return INT_MAX which
+ * may be rejected elsewhere.
  *
  * @param dc		the devclass to examine
  * @param unit		the first unit number to check
@@ -1188,6 +1189,7 @@ devclass_get_sysctl_tree(devclass_t dc)
  * @retval 0		success
  * @retval EEXIST	the requested unit number is already allocated
  * @retval ENOMEM	memory allocation failure
+ * @retval EINVAL	unit is negative or we've run out of units
  */
 static int
 devclass_alloc_unit(devclass_t dc, device_t dev, int *unitp)
@@ -1202,10 +1204,13 @@ devclass_alloc_unit(devclass_t dc, device_t dev, int *unitp)
 		BUS_HINT_DEVICE_UNIT(device_get_parent(dev), dev, dc->name,
 		    &unit);
 
+	/* Unit numbers are either DEVICE_UNIT_ANY or in [0,INT_MAX) */
+	if ((unit < 0 && unit != DEVICE_UNIT_ANY) || unit == INT_MAX)
+		return (EINVAL);
+
 	/* If we were given a wired unit number, check for existing device */
 	if (unit != DEVICE_UNIT_ANY) {
-		if (unit >= 0 && unit < dc->maxunit &&
-		    dc->devices[unit] != NULL) {
+		if (unit < dc->maxunit && dc->devices[unit] != NULL) {
 			if (bootverbose)
 				printf("%s: %s%d already exists; skipping it\n",
 				    dc->name, dc->name, *unitp);
@@ -1214,7 +1219,7 @@ devclass_alloc_unit(devclass_t dc, device_t dev, int *unitp)
 	} else {
 		/* Unwired device, find the next available slot for it */
 		unit = 0;
-		for (unit = 0;; unit++) {
+		for (unit = 0; unit < INT_MAX; unit++) {
 			/* If this device slot is already in use, skip it. */
 			if (unit < dc->maxunit && dc->devices[unit] != NULL)
 				continue;
@@ -1228,6 +1233,15 @@ devclass_alloc_unit(devclass_t dc, device_t dev, int *unitp)
 		}
 	}
 
+	/*
+	 * Unit numbers must be in the range [0, INT_MAX), so exclude INT_MAX as
+	 * too large. We constrain maxunit below to be <= INT_MAX. This means we
+	 * can treat unit and maxunit as normal integers with normal math
+	 * everywhere and we only have to flag INT_MAX as invalid.
+	 */
+	if (unit == INT_MAX)
+		return (EINVAL);
+
 	/*
 	 * We've selected a unit beyond the length of the table, so let's extend
 	 * the table to make room for all units up to and including this one.
@@ -1263,6 +1277,7 @@ devclass_alloc_unit(devclass_t dc, device_t dev, int *unitp)
  * @retval 0		success
  * @retval EEXIST	the requested unit number is already allocated
  * @retval ENOMEM	memory allocation failure
+ * @retval EINVAL	Unit number invalid or too many units
  */
 static int
 devclass_add_device(devclass_t dc, device_t dev)