git: 8b19898a78d5 - main - Fix a panic on boot introduced by 555a861d6826

From: Andrew Gallatin <gallatin_at_FreeBSD.org>
Date: Tue, 01 Nov 2022 17:46:24 UTC
The branch main has been updated by gallatin:

URL: https://cgit.FreeBSD.org/src/commit/?id=8b19898a78d52b351f4d7a6ad1d8b074d037e3b7

commit 8b19898a78d52b351f4d7a6ad1d8b074d037e3b7
Author:     Andrew Gallatin <gallatin@FreeBSD.org>
AuthorDate: 2022-11-01 17:44:39 +0000
Commit:     Andrew Gallatin <gallatin@FreeBSD.org>
CommitDate: 2022-11-01 17:44:39 +0000

    Fix a panic on boot introduced by 555a861d6826
    
    First, an sbuf_new() in device_get_path() shadows the sb
    passed in by dev_wired_cache_add(), leaving its sb in an
    unfinished state, leading to a failed KASSERT().  Fixing this
    is as simple as removing the sbuf_new() from device_get_path()
    
    Second, we cannot simply take a pointer to the sbuf memory and
    store it in the device location cache, because that sbuf
    is freed immediately after we add data to the cache, leading
    to a use-after-free and eventually a double-free.  Fixing this
    requires allocating memory for the path.
    
    After a discussion with jhb, we decided that one malloc was
    better than two in dev_wired_cache_add, which is why it changed
    so much.
    
    Reviewed by: jhb
    Sponsored by: Netflix
    MFC after: 14 days
---
 sys/kern/subr_bus.c | 19 ++++++++++---------
 1 file changed, 10 insertions(+), 9 deletions(-)

diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c
index 5c165419af2d..2fcf650b0289 100644
--- a/sys/kern/subr_bus.c
+++ b/sys/kern/subr_bus.c
@@ -5310,7 +5310,7 @@ device_get_path(device_t dev, const char *locator, struct sbuf *sb)
 	device_t parent;
 	int error;
 
-	sb = sbuf_new(NULL, NULL, 0, SBUF_AUTOEXTEND | SBUF_INCLUDENUL);
+	KASSERT(sb != NULL, ("sb is NULL"));
 	parent = device_get_parent(dev);
 	if (parent == NULL) {
 		error = sbuf_printf(sb, "/");
@@ -5663,8 +5663,6 @@ dev_wired_cache_fini(device_location_cache_t *dcp)
 	struct device_location_node *dln, *tdln;
 
 	TAILQ_FOREACH_SAFE(dln, &dcp->dlc_list, dln_link, tdln) {
-		/* Note: one allocation for both node and locator, but not path */
-		free(__DECONST(void *, dln->dln_path), M_BUS);
 		free(dln, M_BUS);
 	}
 	free(dcp, M_BUS);
@@ -5687,12 +5685,15 @@ static struct device_location_node *
 dev_wired_cache_add(device_location_cache_t *dcp, const char *locator, const char *path)
 {
 	struct device_location_node *dln;
-	char *l;
-
-	dln = malloc(sizeof(*dln) + strlen(locator) + 1, M_BUS, M_WAITOK | M_ZERO);
-	dln->dln_locator = l = (char *)(dln + 1);
-	memcpy(l, locator, strlen(locator) + 1);
-	dln->dln_path = path;
+	size_t loclen, pathlen;
+
+	loclen = strlen(locator) + 1;
+	pathlen = strlen(path) + 1;
+	dln = malloc(sizeof(*dln) + loclen + pathlen, M_BUS, M_WAITOK | M_ZERO);
+	dln->dln_locator = (char *)(dln + 1);
+	memcpy(__DECONST(char *, dln->dln_locator), locator, loclen);
+	dln->dln_path = dln->dln_locator + loclen;
+	memcpy(__DECONST(char *, dln->dln_path), path, pathlen);
 	TAILQ_INSERT_HEAD(&dcp->dlc_list, dln, dln_link);
 
 	return (dln);