From nobody Tue Jun 13 13:54:24 2023 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 4QgVR44t24z4cV8R; Tue, 13 Jun 2023 13:54:24 +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 "R3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 4QgVR43dbgz3y8F; Tue, 13 Jun 2023 13:54:24 +0000 (UTC) (envelope-from git@FreeBSD.org) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1686664464; 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=TuUUxKpWsEjOvDDhMgs3yxxgfu9ak2y12/TeUHheT5I=; b=Q9VGqCy+zYkQ2VQBQWmcq/aR03FxMEszMnBitUkSLjvrfm4j1m5XU9Sr5nxUv5jsAAdluQ 7Ekvz0NNXjM4endNhQrftmW5yGv3BpPD+EJbuK4YRlGLrJb0plgFA7CW67rDBhGazGwzo6 1Pl2L8kyjRyb91NFIFknVg8qAuo3nk2bI8QP8kQxHxBVngwmIQhac183HhzFy2EbvaW06g u2xn8YOXm7rET9ncTFhI/vZg5Msuktt21pDSzrXTJXsQwIsdaJEXRNbiBeA3XXAwlfolRX df5uyaMvtRrk2QsTw5Bk8dv2VX2d6fwNSBZ6OJYftSKeE9qm3PyusO6ttDI6cQ== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=freebsd.org; s=dkim; t=1686664464; 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=TuUUxKpWsEjOvDDhMgs3yxxgfu9ak2y12/TeUHheT5I=; b=CnbnCeRjNJAK1nm7knW4xkMLPxe73VpupSp+aJ//tsoz+/IPbIKQBSisRvWY7oqJ2M7i4h bPMdOocknuvALM1TQQtwZkRlsO4DlUiVFfUAFZynzaMRTU4OBcjm/p4Lh9u1XG9zxjTAdU 9bZsE7DU7IEWOIJIpXmg9OCGPpkEXRjEAyshlwr7s4mY+T9d045gSYrpeSfmcAAIPQXSnI A67AeHN3SLjx7zEoM5gDzaaTPdCsjtfcVr/GvPdi5SgrTvy1uT2rW3K7RaNvFomWrpRjBq zH+RwlJeY9p0rR3xCNNIEkhL8CYXmmvCvnpy+abrA5T+VpFpLrcTj4gkirQ2pw== ARC-Authentication-Results: i=1; mx1.freebsd.org; none ARC-Seal: i=1; s=dkim; d=freebsd.org; t=1686664464; a=rsa-sha256; cv=none; b=Likmmc8nxJABO8wWjgdIeUYusaJ1PVf3PFuNMWDJpw6cpKqvI/uAHpWyK8S/KXKZmsq+yD q+c5uUirpoiafJNTE6x/pDKAAI7rSw0ShfybcI8d+Pn3Q0aHTVfsX/oCZO98cnmmVicl9P azqWTJL6Vchf2v8NxNBNTgBWr3YMCVqtGozxBQUbTAhMT+guGxTyJKkIRFfNTv2CWg1pE7 iIIGZ9/CkLr+c1kiDdSytMx4bKwQVf4Rq4GmvkBveb/PtykdVgVNi1SA8RELTrr4gaPBq9 UnBECAy2B9QKUibty9u0agXKf1MdNNRnWcsUV3WR6gyOi0os8YlWAS9GQEcNmQ== 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 4QgVR42GK4zPtj; Tue, 13 Jun 2023 13:54:24 +0000 (UTC) (envelope-from git@FreeBSD.org) Received: from gitrepo.freebsd.org ([127.0.1.44]) by gitrepo.freebsd.org (8.16.1/8.16.1) with ESMTP id 35DDsOHl073416; Tue, 13 Jun 2023 13:54:24 GMT (envelope-from git@gitrepo.freebsd.org) Received: (from git@localhost) by gitrepo.freebsd.org (8.16.1/8.16.1/Submit) id 35DDsOq2073415; Tue, 13 Jun 2023 13:54:24 GMT (envelope-from git) Date: Tue, 13 Jun 2023 13:54:24 GMT Message-Id: <202306131354.35DDsOq2073415@gitrepo.freebsd.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org From: Kristof Provost Subject: git: 0ba9cb5e710f - main - dummynet: fix wf2q use-after-free 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: Sender: owner-dev-commits-src-all@freebsd.org X-BeenThere: dev-commits-src-all@freebsd.org MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 8bit X-Git-Committer: kp X-Git-Repository: src X-Git-Refname: refs/heads/main X-Git-Reftype: branch X-Git-Commit: 0ba9cb5e710f42fcbc5d710a606bfae5a7f90984 Auto-Submitted: auto-generated X-ThisMailContainsUnwantedMimeParts: N The branch main has been updated by kp: URL: https://cgit.FreeBSD.org/src/commit/?id=0ba9cb5e710f42fcbc5d710a606bfae5a7f90984 commit 0ba9cb5e710f42fcbc5d710a606bfae5a7f90984 Author: Kristof Provost AuthorDate: 2023-06-12 13:05:41 +0000 Commit: Kristof Provost CommitDate: 2023-06-13 13:51:47 +0000 dummynet: fix wf2q use-after-free When we clean up a wf2q+ queue we need to ensure that we remove it from the correct heap. If we leave a queue pointer behind in an unexpected heap we'll later write to it, causing a use-after-free and unpredictable panics. Teach the dummynet heap code to verify that we're removing the correct object so we can safely attempt to remove objects not contained in the heap. Remove a to-be-removed queue from all heaps. Also don't continue the enqueue function if we're not finding the queue on the idle heap as we'd expect. While here also remove the empty heap warning, because this is now expected to happen. See also: https://redmine.pfsense.org/issues/14433 Sponsored by: Rubicon Communications, LLC ("Netgate") --- sys/netpfil/ipfw/dn_heap.c | 18 +++++++++++------- sys/netpfil/ipfw/dn_heap.h | 2 +- sys/netpfil/ipfw/dn_sched_wf2q.c | 14 ++++++-------- 3 files changed, 18 insertions(+), 16 deletions(-) diff --git a/sys/netpfil/ipfw/dn_heap.c b/sys/netpfil/ipfw/dn_heap.c index d1c386e48697..bc6b9c142dc2 100644 --- a/sys/netpfil/ipfw/dn_heap.c +++ b/sys/netpfil/ipfw/dn_heap.c @@ -178,14 +178,13 @@ heap_insert(struct dn_heap *h, uint64_t key1, void *p) /* * remove top element from heap, or obj if obj != NULL */ -void +bool heap_extract(struct dn_heap *h, void *obj) { int child, father, max = h->elements - 1; if (max < 0) { - printf("--- %s: empty heap 0x%p\n", __FUNCTION__, h); - return; + return false; } if (obj == NULL) father = 0; /* default: move up smallest child */ @@ -194,11 +193,14 @@ heap_extract(struct dn_heap *h, void *obj) panic("%s: extract from middle not set on %p\n", __FUNCTION__, h); father = *((int *)((char *)obj + h->ofs)); - if (father < 0 || father >= h->elements) { - panic("%s: father %d out of bound 0..%d\n", - __FUNCTION__, father, h->elements); - } + if (father < 0 || father >= h->elements) + return false; } + /* We should make sure that the object we're trying to remove is + * actually in this heap. */ + if (obj != NULL && h->p[father].object != obj) + return false; + /* * below, father is the index of the empty element, which * we replace at each step with the smallest child until we @@ -223,6 +225,8 @@ heap_extract(struct dn_heap *h, void *obj) h->p[father] = h->p[max]; heap_insert(h, father, NULL); } + + return true; } #if 0 diff --git a/sys/netpfil/ipfw/dn_heap.h b/sys/netpfil/ipfw/dn_heap.h index f50ffdef33eb..4ae1c1cb8750 100644 --- a/sys/netpfil/ipfw/dn_heap.h +++ b/sys/netpfil/ipfw/dn_heap.h @@ -102,7 +102,7 @@ enum { #define SET_HEAP_OFS(h, n) do { (h)->ofs = n; } while (0) int heap_init(struct dn_heap *h, int size, int ofs); int heap_insert(struct dn_heap *h, uint64_t key1, void *p); -void heap_extract(struct dn_heap *h, void *obj); +bool heap_extract(struct dn_heap *h, void *obj); void heap_free(struct dn_heap *h); int heap_scan(struct dn_heap *, int (*)(void *, uintptr_t), uintptr_t); diff --git a/sys/netpfil/ipfw/dn_sched_wf2q.c b/sys/netpfil/ipfw/dn_sched_wf2q.c index 7f81c0a098f1..5b69eb083d7f 100644 --- a/sys/netpfil/ipfw/dn_sched_wf2q.c +++ b/sys/netpfil/ipfw/dn_sched_wf2q.c @@ -157,7 +157,8 @@ wf2qp_enqueue(struct dn_sch_inst *_si, struct dn_queue *q, struct mbuf *m) si->wsum += fs->fs.par[0]; /* add weight of new queue. */ si->inv_wsum = ONE_FP/si->wsum; } else { /* if it was idle then it was in the idle heap */ - heap_extract(&si->idle_heap, q); + if (! heap_extract(&si->idle_heap, q)) + return 1; alg_fq->S = MAX64(alg_fq->F, si->V); /* compute new S */ } alg_fq->F = alg_fq->S + len * alg_fq->inv_w; @@ -338,13 +339,10 @@ wf2qp_free_queue(struct dn_queue *q) /* extract from the heap. XXX TODO we may need to adjust V * to make sure the invariants hold. */ - if (q->mq.head == NULL) { - heap_extract(&si->idle_heap, q); - } else if (DN_KEY_LT(si->V, alg_fq->S)) { - heap_extract(&si->ne_heap, q); - } else { - heap_extract(&si->sch_heap, q); - } + heap_extract(&si->idle_heap, q); + heap_extract(&si->ne_heap, q); + heap_extract(&si->sch_heap, q); + return 0; }