From patchwork Tue Sep 24 15:45:07 2024 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Ian Whitfield X-Patchwork-Id: 1988996 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@legolas.ozlabs.org Authentication-Results: legolas.ozlabs.org; spf=pass (sender SPF authorized) smtp.mailfrom=lists.ubuntu.com (client-ip=185.125.189.65; helo=lists.ubuntu.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=patchwork.ozlabs.org) Received: from lists.ubuntu.com (lists.ubuntu.com [185.125.189.65]) (using TLSv1.2 with cipher ECDHE-ECDSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by legolas.ozlabs.org (Postfix) with ESMTPS id 4XCkhg0YKTz1xsw for ; Wed, 25 Sep 2024 01:45:22 +1000 (AEST) Received: from localhost ([127.0.0.1] helo=lists.ubuntu.com) by lists.ubuntu.com with esmtp (Exim 4.86_2) (envelope-from ) id 1st7iz-0006eO-5k; Tue, 24 Sep 2024 15:45:13 +0000 Received: from smtp-relay-internal-0.internal ([10.131.114.225] helo=smtp-relay-internal-0.canonical.com) by lists.ubuntu.com with esmtps (TLS1.2:ECDHE_RSA_AES_128_GCM_SHA256:128) (Exim 4.86_2) (envelope-from ) id 1st7ix-0006e0-2k for kernel-team@lists.ubuntu.com; Tue, 24 Sep 2024 15:45:11 +0000 Received: from mail-oa1-f71.google.com (mail-oa1-f71.google.com [209.85.160.71]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by smtp-relay-internal-0.canonical.com (Postfix) with ESMTPS id B5F173F1F4 for ; Tue, 24 Sep 2024 15:45:10 +0000 (UTC) Received: by mail-oa1-f71.google.com with SMTP id 586e51a60fabf-278222c9c13so3077556fac.2 for ; Tue, 24 Sep 2024 08:45:10 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1727192709; x=1727797509; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=3EkBHjR8kMDHYt2WZI7BUSE0Yi0b6JbE4/4S/1jOfKw=; b=Pipljw4UQ3t2kvRm4bF0dKeLFYXri6y8GpppFMbn8p0Is6kI9hz/3YIostuGYI/QaZ XQPsWV+jzBFxNdQZpplkeCfQ4xbTzbBnVcah7efLQk+vzzLya3yVOPKXkCsa7zuDq/1Q 1CajpPAK0EE5dDFm/OF5BqX2wvtWr+5RmtomhprO+0SWPsVnHcu0hXZRkE5XSsBF+bDj 97R0w7hwJDIg3zdlAHKJUrbIpDFVezRDnxGIaHuV2G4SYn7oPqPLqzOTu5KyOMFXkd0s lodZGlmz/i17PTihzq5Pk8usR3RMI5Gi0j4c1XCCCuNBTt8dLUtGE3uC+UWLXkEM+m6l opHg== X-Gm-Message-State: AOJu0YxbN00cIPl7fGc9F3hRTmpBsDRWAMmgN0Emz4ZKpIvxO+9SkzIv VbOeoEQ6KEtdr1q2gZwwamvl1tn7O5ZbDKEi5UkwnEXMPdnd5QMaWd8p4D4nDRSMGgmTU8RAlnx wNNvBAZmuZZeMVj7Y3yAlWQZ2D0F+u6CSw9dgh2ETpenJa1cVo059Njjk+cgo14+yX0bfVk0pf8 oiTpQFsAuR5g== X-Received: by 2002:a05:6870:5b88:b0:277:eea4:a436 with SMTP id 586e51a60fabf-2803a57f035mr9611250fac.7.1727192709436; Tue, 24 Sep 2024 08:45:09 -0700 (PDT) X-Google-Smtp-Source: AGHT+IFRypOQ9YVjiENNarW8tEMapuGVFOB4JxsIjezxFOpBurjpWz9vqzUq8tsq+hr7nrbTcDCRiA== X-Received: by 2002:a05:6870:5b88:b0:277:eea4:a436 with SMTP id 586e51a60fabf-2803a57f035mr9611217fac.7.1727192708957; Tue, 24 Sep 2024 08:45:08 -0700 (PDT) Received: from localhost ([2600:1700:3ec0:2680:172f:739a:7010:5be6]) by smtp.gmail.com with ESMTPSA id 586e51a60fabf-283af93833dsm592530fac.25.2024.09.24.08.45.08 for (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 24 Sep 2024 08:45:08 -0700 (PDT) From: Ian Whitfield To: kernel-team@lists.ubuntu.com Subject: [SRU][N][PATCH 1/1] netem: fix return value if duplicate enqueue fails Date: Tue, 24 Sep 2024 08:45:07 -0700 Message-ID: <20240924154507.14124-2-ian.whitfield@canonical.com> X-Mailer: git-send-email 2.43.0 In-Reply-To: <20240924154507.14124-1-ian.whitfield@canonical.com> References: <20240924154507.14124-1-ian.whitfield@canonical.com> MIME-Version: 1.0 X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: Stephen Hemminger There is a bug in netem_enqueue() introduced by commit 5845f706388a ("net: netem: fix skb length BUG_ON in __skb_to_sgvec") that can lead to a use-after-free. This commit made netem_enqueue() always return NET_XMIT_SUCCESS when a packet is duplicated, which can cause the parent qdisc's q.qlen to be mistakenly incremented. When this happens qlen_notify() may be skipped on the parent during destruction, leaving a dangling pointer for some classful qdiscs like DRR. There are two ways for the bug happen: - If the duplicated packet is dropped by rootq->enqueue() and then the original packet is also dropped. - If rootq->enqueue() sends the duplicated packet to a different qdisc and the original packet is dropped. In both cases NET_XMIT_SUCCESS is returned even though no packets are enqueued at the netem qdisc. The fix is to defer the enqueue of the duplicate packet until after the original packet has been guaranteed to return NET_XMIT_SUCCESS. Fixes: 5845f706388a ("net: netem: fix skb length BUG_ON in __skb_to_sgvec") Reported-by: Budimir Markovic Signed-off-by: Stephen Hemminger Reviewed-by: Simon Horman Link: https://patch.msgid.link/20240819175753.5151-1-stephen@networkplumber.org Signed-off-by: Jakub Kicinski (cherry picked from commit c07ff8592d57ed258afee5a5e04991a48dbaf382) CVE-2024-45016 Signed-off-by: Ian Whitfield --- net/sched/sch_netem.c | 47 ++++++++++++++++++++++++++----------------- 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/net/sched/sch_netem.c b/net/sched/sch_netem.c index fa678eb88528..0d29d0ccf8cb 100644 --- a/net/sched/sch_netem.c +++ b/net/sched/sch_netem.c @@ -446,12 +446,10 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch, struct netem_sched_data *q = qdisc_priv(sch); /* We don't fill cb now as skb_unshare() may invalidate it */ struct netem_skb_cb *cb; - struct sk_buff *skb2; + struct sk_buff *skb2 = NULL; struct sk_buff *segs = NULL; unsigned int prev_len = qdisc_pkt_len(skb); int count = 1; - int rc = NET_XMIT_SUCCESS; - int rc_drop = NET_XMIT_DROP; /* Do not fool qdisc_drop_all() */ skb->prev = NULL; @@ -480,19 +478,11 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch, skb_orphan_partial(skb); /* - * If we need to duplicate packet, then re-insert at top of the - * qdisc tree, since parent queuer expects that only one - * skb will be queued. + * If we need to duplicate packet, then clone it before + * original is modified. */ - if (count > 1 && (skb2 = skb_clone(skb, GFP_ATOMIC)) != NULL) { - struct Qdisc *rootq = qdisc_root_bh(sch); - u32 dupsave = q->duplicate; /* prevent duplicating a dup... */ - - q->duplicate = 0; - rootq->enqueue(skb2, rootq, to_free); - q->duplicate = dupsave; - rc_drop = NET_XMIT_SUCCESS; - } + if (count > 1) + skb2 = skb_clone(skb, GFP_ATOMIC); /* * Randomized packet corruption. @@ -504,7 +494,8 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch, if (skb_is_gso(skb)) { skb = netem_segment(skb, sch, to_free); if (!skb) - return rc_drop; + goto finish_segs; + segs = skb->next; skb_mark_not_on_list(skb); qdisc_skb_cb(skb)->pkt_len = skb->len; @@ -530,7 +521,24 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch, /* re-link segs, so that qdisc_drop_all() frees them all */ skb->next = segs; qdisc_drop_all(skb, sch, to_free); - return rc_drop; + if (skb2) + __qdisc_drop(skb2, to_free); + return NET_XMIT_DROP; + } + + /* + * If doing duplication then re-insert at top of the + * qdisc tree, since parent queuer expects that only one + * skb will be queued. + */ + if (skb2) { + struct Qdisc *rootq = qdisc_root_bh(sch); + u32 dupsave = q->duplicate; /* prevent duplicating a dup... */ + + q->duplicate = 0; + rootq->enqueue(skb2, rootq, to_free); + q->duplicate = dupsave; + skb2 = NULL; } qdisc_qstats_backlog_inc(sch, skb); @@ -601,9 +609,12 @@ static int netem_enqueue(struct sk_buff *skb, struct Qdisc *sch, } finish_segs: + if (skb2) + __qdisc_drop(skb2, to_free); + if (segs) { unsigned int len, last_len; - int nb; + int rc, nb; len = skb ? skb->len : 0; nb = skb ? 1 : 0;