git: 055b41056ef7 - main - newbus: Limit units to [0, INT_MAX)
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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)