git: 8b19898a78d5 - main - Fix a panic on boot introduced by 555a861d6826
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
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);