From nobody Fri Nov 01 17:55:49 2024 X-Original-To: dev-commits-src-all@mlmmj.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mlmmj.nyi.freebsd.org (Postfix) with ESMTP id 4Xg7nd5qC7z5cBmm; Fri, 01 Nov 2024 17:55:49 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "R10" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4Xg7nd5Byjz4FBL; Fri, 1 Nov 2024 17:55:49 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1730483749; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=uCm0P6gW47h0/wQ2zX96z8OHSJXL8Xb88s/6LuJiBJI=; b=eC17FckG7GOqdPZRZxGFvu4lkbHFXk1JsQVYPgwkbIEvbJi+gM3r68nJk9s3TsKWs67IFW kOgtjeIyZpGS74H6TW1VsY11hWBTOf6pFyP8FKZ0VocASyL+NVRyD94J7tIVDfcVMfGTMk bqXoqiZ9rhrEhEW5xKbr+5iv7891OyNdtCiMuo2ly8E1EG0aKlZdeOjA4wvUshSheer4r6 P9F1ZMjrgNiBmYMEfnpY/ly6kOAS5DrtTdQm2Zf3mbMr1oGpxvB64PBY7YY3UXwPr9RiCR RhcicLBKVdScb0FWzcdBRrh8t+emcCyNiJ3GbzvdySeLiVjt50y+6/+KHpu55A== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1730483749; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding; bh=uCm0P6gW47h0/wQ2zX96z8OHSJXL8Xb88s/6LuJiBJI=; b=AWp9RGomwu8HllvZ2RsKuD9HRnedzbnUd0TZgqGrzOqVYiYtraEg5AVzM6vS7Kq40hBsjx 5Qeun56RbaJr5oUNmIK/AKEuA7bEvXeSd9UAqI107paRBgU4RpZSbqqp7xhnlP6GK7ZrUV AzZtatuQTYARtY7mwOCBhrMF1qAnFd77CTNsf7v+ScuuiRuMdRSMYW45e8MzfgWvkehrI5 cviONCHGlvb458bXKsTdcH0DKHisj495ETGrb9hVBCoTaV2EGv3KzSPuZRmC1d0pOX9nsg UVAHxgRsE/ZWDgJeLUqB4YQVAnfZK5g29U2v27Ma80cQq11IEHvh3ritQ8UoHw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1730483749; a=rsa-sha256; cv=none; b=YbF2icHwy3DCyFgAmNkPo2Qti1sFzxUXfglr5SrLozgmGbnAwpvmlj4W8rP+WI+ylkLEBp S+QXLsoT0268plxTwQyKEQ9wGytQVhgCMxXSJ4a2Rh5Fi5OQdp83OY44TR/Pm22VFFnszr k+yfcczSCH6x2V4woOcTLy9nMIDMtJf5JsIesmFpikNfW5N36VDoKVRf0tp27TsYbeVU1X +JwohwgS6U6ibDTk5jKYnKKmJwPTpBqfFcniQHSddmHYpKJhk9lu8ozral4XB2pMavgf+c Db0WJzK8CQyWBBBwsVe8DpIt+ZDIrSET0cgiiUQuSIognzC2iGHcoq/QwFwbag== Received: from gitrepo.freebsd.org (gitrepo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:5]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256) (Client did not present a certificate) by mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id 4Xg7nd4plxzWFn; Fri, 1 Nov 2024 17:55:49 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.18.1/8.18.1) with ESMTP id 4A1HtnBm072548; Fri, 1 Nov 2024 17:55:49 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.18.1/8.18.1/Submit) id 4A1Htn6t072545; Fri, 1 Nov 2024 17:55:49 GMT (envelope-from git) Date: Fri, 1 Nov 2024 17:55:49 GMT Message-Id: <202411011755.4A1Htn6t072545@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Warner Losh Subject: git: 055b41056ef7 - main - newbus: Limit units to [0, INT_MAX) List-Id: Commit messages for all branches of the src repository List-Archive: https://lists.freebsd.org/archives/dev-commits-src-all List-Help: List-Post: List-Subscribe: List-Unsubscribe: X-BeenThere: dev-commits-src-all@freebsd.org Sender: owner-dev-commits-src-all@FreeBSD.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: imp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 055b41056ef7a54d0a75ba5c9049fc0bd34a8b26 Auto-Submitted: auto-generated The branch main has been updated by imp: URL: https://cgit.FreeBSD.org/src/commit/?id=055b41056ef7a54d0a75ba5c9049fc0bd34a8b26 commit 055b41056ef7a54d0a75ba5c9049fc0bd34a8b26 Author: Warner Losh AuthorDate: 2024-10-31 22:50:54 +0000 Commit: Warner Losh 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 --- 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)