From nobody Sat Feb 7 21:16:09 2026 Received: from mail-pj1-f49.google.com (mail-pj1-f49.google.com [209.85.216.49]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 36AD618757F for ; Mon, 19 Aug 2024 17:58:03 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.216.49 ARC-Seal: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724090286; cv=none; b=NouzriSXsvAh0PVbEJZ/elXc9S5xwpMWxUpQIOIa9BD8yA5jCg0EXmmkT5hEC6FTlDm+8Ks1IR4+YKBjpKPzE7eTGsdE7lPb/mKdDyUvrGvpPG5XCZeoiilzQraaIT1WMJCu5Z1nbsvEHCbY+gkiORVKs3wWz8uXoW/0VHyqI+U= ARC-Message-Signature: i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1724090286; c=relaxed/simple; bh=I74Nsd/hv/7Z1KBPIywZQhxoLRuZY2jSJNhcyK2ch8A=; h=From:To:Cc:Subject:Date:Message-ID:MIME-Version; b=LLqz12t/nJS89AbfObeQOTMdoh7wr+qw207Lct52+Uqm8MtgoeOnVy1oQD+OJzGd3IKW4LKKZVHpl1DXLmq1xoxfkOosFT8blK/di0rp22nTJ+0nMD8szoKVdyKGivwIIeydnWbKg20yuBRZib32kIxVKt1pUSInPqTBuRMR8Ls= ARC-Authentication-Results: i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=networkplumber.org; spf=pass smtp.mailfrom=networkplumber.org; dkim=pass (2048-bit key) header.d=networkplumber-org.20230601.gappssmtp.com header.i=@networkplumber-org.20230601.gappssmtp.com header.b=mSSIx/59; arc=none smtp.client-ip=209.85.216.49 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=networkplumber.org Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=networkplumber.org Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=networkplumber-org.20230601.gappssmtp.com header.i=@networkplumber-org.20230601.gappssmtp.com header.b="mSSIx/59" Received: by mail-pj1-f49.google.com with SMTP id 98e67ed59e1d1-2d3d662631aso3065558a91.1 for ; Mon, 19 Aug 2024 10:58:03 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20230601.gappssmtp.com; s=20230601; t=1724090283; x=1724695083; darn=vger.kernel.org; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:from:to:cc:subject:date:message-id:reply-to; bh=fj/JRnUiS2ccvYLpltbwZnQSUIfTnOEAJgrq5esozr0=; b=mSSIx/598POFlBCVGOMvkDcO4hFH/qqTIQZ9RBqX+vygWbn+HPVV0oDhcYz34zFWeQ pY7FvkRY8dBY8mbRIqBiszOZerQbc92S/aIJRDBzzlsfPNLpa4HFwoVf+n03mIL1pwOs 2/lp9g/eZNlS09/H+gy+xxs+acwd+l7L3e/wfnuj9t6JludxO9s8I6HpSS+lVX8Z4UEo 8QT3LbYX00SaP5zcBSUV2neoGDzZxK6nbtnILDMsgwt4A9u52w/huxF9RfeZV+UFdTTq 2DQDCrvoW+Hpm/Zuy3HNV8bvZizHjgHa9l04hNADXVlYRnu9iaqIe4mMdYlto0/2Njgz 3JCg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1724090283; x=1724695083; h=content-transfer-encoding:mime-version:message-id:date:subject:cc :to:from:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=fj/JRnUiS2ccvYLpltbwZnQSUIfTnOEAJgrq5esozr0=; b=ZASKeRfY4Kmglh4rX8VeLuZ+hU9TdTB8jhSgvbknjtuiTr6EHOmKa6itKikVSllkXL BwXo9OGZd045tPqJEtfoZmvDQ+YNw6vWe18UcwZuX8ybdjGYonP25mhYaldVa6Mh4zVX MeCdYwdog8NWNlR1sC49wzJiFCmUSLpDf3KRZZRnBSoeiGBnAZyeGih2jRumabN3ud57 TC0NDnmJiTodG30jONOsijDFxU8KNWCPFj1+aWu/jJkPeNUExj0V+8i3aVERdwhZxXh0 JOCTtZp0QsXYY+D28nuXzxI4xWwZvkNkN1by7Hjm5N4uUXMSe7DDPYUU0DrbfP1vYeeU EbvQ== X-Forwarded-Encrypted: i=1; AJvYcCXoGs2NpwlEGfhjWgCYkSduJ/G4jlmm/Rvs+lcAu1k2C//M/noSCQEpK3vDoMxWKoWniY3K92K1jYERUdj7aCnuqbptqILyMn4MJhly X-Gm-Message-State: AOJu0YyAcoD8Drf82/fvt68naQIUrf7M9zPDVYaTZ/yEcRryNybda1hw A29dK4+mRfQKsWFncocVNxA/5FSZxCDvbzpG6cQdpMZpPjq0CpeDl7nniWTgd/k= X-Google-Smtp-Source: AGHT+IFgWVEx6cbuYxIpkjpE7FNl342NnFixb6XDIp1xkcfwI2POSDxplq/p6uHN0KhZi8o+IkJkOg== X-Received: by 2002:a17:90a:db86:b0:2ca:5ec8:576c with SMTP id 98e67ed59e1d1-2d3dfda7e42mr12432690a91.5.1724090283352; Mon, 19 Aug 2024 10:58:03 -0700 (PDT) Received: from hermes.local (204-195-96-226.wavecable.com. [204.195.96.226]) by smtp.gmail.com with ESMTPSA id 98e67ed59e1d1-2d3c0b87dc8sm9635504a91.38.2024.08.19.10.58.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 19 Aug 2024 10:58:02 -0700 (PDT) From: Stephen Hemminger To: netdev@vger.kernel.org Cc: Stephen Hemminger , Budimir Markovic , Jamal Hadi Salim , Cong Wang , Jiri Pirko , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Sheng Lan , linux-kernel@vger.kernel.org (open list) Subject: [PATCH] netem: fix return value if duplicate enqueue fails Date: Mon, 19 Aug 2024 10:56:45 -0700 Message-ID: <20240819175753.5151-1-stephen@networkplumber.org> X-Mailer: git-send-email 2.43.0 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Transfer-Encoding: quoted-printable Content-Type: text/plain; charset="utf-8" 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 t= he parent during destruction, leaving a dangling pointer for some classful qdi= scs like DRR. There are two ways for the bug happen: - If the duplicated packet is dropped by rootq->enqueue() and then the orig= inal 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 enque= ued 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 --- 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 edc72962ae63..0f8d581438c3 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 =3D 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 =3D NULL; struct sk_buff *segs =3D NULL; unsigned int prev_len =3D qdisc_pkt_len(skb); int count =3D 1; - int rc =3D NET_XMIT_SUCCESS; - int rc_drop =3D NET_XMIT_DROP; =20 /* Do not fool qdisc_drop_all() */ skb->prev =3D NULL; @@ -480,19 +478,11 @@ static int netem_enqueue(struct sk_buff *skb, struct = Qdisc *sch, skb_orphan_partial(skb); =20 /* - * 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 =3D skb_clone(skb, GFP_ATOMIC)) !=3D NULL) { - struct Qdisc *rootq =3D qdisc_root_bh(sch); - u32 dupsave =3D q->duplicate; /* prevent duplicating a dup... */ - - q->duplicate =3D 0; - rootq->enqueue(skb2, rootq, to_free); - q->duplicate =3D dupsave; - rc_drop =3D NET_XMIT_SUCCESS; - } + if (count > 1) + skb2 =3D skb_clone(skb, GFP_ATOMIC); =20 /* * Randomized packet corruption. @@ -504,7 +494,8 @@ static int netem_enqueue(struct sk_buff *skb, struct Qd= isc *sch, if (skb_is_gso(skb)) { skb =3D netem_segment(skb, sch, to_free); if (!skb) - return rc_drop; + goto finish_segs; + segs =3D skb->next; skb_mark_not_on_list(skb); qdisc_skb_cb(skb)->pkt_len =3D skb->len; @@ -530,7 +521,24 @@ static int netem_enqueue(struct sk_buff *skb, struct Q= disc *sch, /* re-link segs, so that qdisc_drop_all() frees them all */ skb->next =3D 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 =3D qdisc_root_bh(sch); + u32 dupsave =3D q->duplicate; /* prevent duplicating a dup... */ + + q->duplicate =3D 0; + rootq->enqueue(skb2, rootq, to_free); + q->duplicate =3D dupsave; + skb2 =3D NULL; } =20 qdisc_qstats_backlog_inc(sch, skb); @@ -601,9 +609,12 @@ static int netem_enqueue(struct sk_buff *skb, struct Q= disc *sch, } =20 finish_segs: + if (skb2) + __qdisc_drop(skb2, to_free); + if (segs) { unsigned int len, last_len; - int nb; + int rc, nb; =20 len =3D skb ? skb->len : 0; nb =3D skb ? 1 : 0; --=20 2.43.0