From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id B2817A04DD for ; Wed, 18 Nov 2020 17:37:26 +0100 (CET) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 96E42C904; Wed, 18 Nov 2020 17:37:23 +0100 (CET) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [63.128.21.124]) by dpdk.org (Postfix) with ESMTP id 6F7C2C8F8 for ; Wed, 18 Nov 2020 17:37:20 +0100 (CET) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1605717439; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=0COLTR5ZmMmpD+zaklo4m+tcQZOPmaGtLI4kiTEBcyE=; b=SAfmq++s19kveH6uHK/uIMGoF63T9T4lY6IO0Zrqa/PMAj9O5bYZbwRZtL5h6FF+6oLF6z dd+WJlGeuKbYFbYYzuJ51bMJ/PhvrhbljtXkdXDwQ32QmvtyfvHyzYQIvFDpqltlB/snlv wyU+TV85Qq5JnfKnacQKp+BllEkEdis= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-386-pVfgklcxP3eJUyVOToERfg-1; Wed, 18 Nov 2020 11:37:16 -0500 X-MC-Unique: pVfgklcxP3eJUyVOToERfg-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.phx2.redhat.com [10.5.11.16]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id C2BBFA4841; Wed, 18 Nov 2020 16:36:51 +0000 (UTC) Received: from rh.redhat.com (ovpn-113-249.ams2.redhat.com [10.36.113.249]) by smtp.corp.redhat.com (Postfix) with ESMTP id C37E85C1A3; Wed, 18 Nov 2020 16:36:50 +0000 (UTC) From: Kevin Traynor To: Lukasz Wojciechowski Cc: David Hunt , dpdk stable Date: Wed, 18 Nov 2020 16:35:06 +0000 Message-Id: <20201118163558.1101823-20-ktraynor@redhat.com> In-Reply-To: <20201118163558.1101823-1-ktraynor@redhat.com> References: <20201118163558.1101823-1-ktraynor@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.16 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=ktraynor@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: 8bit Content-Type: text/plain; charset="US-ASCII" Subject: [dpdk-stable] patch 'distributor: fix buffer use after free' has been queued to LTS release 18.11.11 X-BeenThere: stable@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: patches for DPDK stable branches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" Hi, FYI, your patch has been queued to LTS release 18.11.11 Note it hasn't been pushed to http://dpdk.org/browse/dpdk-stable yet. It will be pushed if I get no objections before 11/24/20. So please shout if anyone has objections. Also note that after the patch there's a diff of the upstream commit vs the patch applied to the branch. This will indicate if there was any rebasing needed to apply to the stable branch. If there were code changes for rebasing (ie: not only metadata diffs), please double check that the rebase was correctly done. Queued patches are on a temporary branch at: https://github.com/kevintraynor/dpdk-stable-queue This queued commit can be viewed at: https://github.com/kevintraynor/dpdk-stable-queue/commit/de6e48d0b37ccb04376af977abc27c2833f7364e Thanks. Kevin. --- >From de6e48d0b37ccb04376af977abc27c2833f7364e Mon Sep 17 00:00:00 2001 From: Lukasz Wojciechowski Date: Sat, 17 Oct 2020 05:06:47 +0200 Subject: [PATCH] distributor: fix buffer use after free [ upstream commit 6bd951b48222caaa10a796057f617cab04f928b0 ] rte_distributor_request_pkt and rte_distributor_get_pkt dereferenced oldpkt parameter when in RTE_DIST_ALG_SINGLE even if number of returned buffers from worker to distributor was 0. This patch passes NULL to the legacy API when number of returned buffers is 0. This allows passing NULL as oldpkt parameter. Distributor tests are also updated passing NULL as oldpkt and 0 as number of returned packets, where packets are not returned. Fixes: 775003ad2f96 ("distributor: add new burst-capable library") Signed-off-by: Lukasz Wojciechowski Acked-by: David Hunt --- lib/librte_distributor/rte_distributor.c | 4 ++-- test/test/test_distributor.c | 28 +++++++++--------------- 2 files changed, 12 insertions(+), 20 deletions(-) diff --git a/lib/librte_distributor/rte_distributor.c b/lib/librte_distributor/rte_distributor.c index d1f8b14d2e..5f9dde8494 100644 --- a/lib/librte_distributor/rte_distributor.c +++ b/lib/librte_distributor/rte_distributor.c @@ -44,5 +44,5 @@ rte_distributor_request_pkt_v1705(struct rte_distributor *d, if (unlikely(d->alg_type == RTE_DIST_ALG_SINGLE)) { rte_distributor_request_pkt_v20(d->d_v20, - worker_id, oldpkt[0]); + worker_id, count ? oldpkt[0] : NULL); return; } @@ -144,5 +144,5 @@ rte_distributor_get_pkt_v1705(struct rte_distributor *d, if (return_count <= 1) { pkts[0] = rte_distributor_get_pkt_v20(d->d_v20, - worker_id, oldpkt[0]); + worker_id, return_count ? oldpkt[0] : NULL); return (pkts[0]) ? 1 : 0; } else diff --git a/test/test/test_distributor.c b/test/test/test_distributor.c index 6aa4e49e69..ae7eff5a91 100644 --- a/test/test/test_distributor.c +++ b/test/test/test_distributor.c @@ -63,11 +63,8 @@ handle_work(void *arg) struct worker_params *wp = arg; struct rte_distributor *db = wp->dist; - unsigned int count = 0, num = 0; + unsigned int count = 0, num; unsigned int id = __sync_fetch_and_add(&worker_idx, 1); - int i; - for (i = 0; i < 8; i++) - buf[i] = NULL; - num = rte_distributor_get_pkt(db, id, buf, buf, num); + num = rte_distributor_get_pkt(db, id, buf, NULL, 0); while (!quit) { __atomic_fetch_add(&worker_stats[id].handled_packets, num, @@ -273,10 +270,8 @@ handle_work_with_free_mbufs(void *arg) unsigned int count = 0; unsigned int i; - unsigned int num = 0; + unsigned int num; unsigned int id = __sync_fetch_and_add(&worker_idx, 1); - for (i = 0; i < 8; i++) - buf[i] = NULL; - num = rte_distributor_get_pkt(d, id, buf, buf, num); + num = rte_distributor_get_pkt(d, id, buf, NULL, 0); while (!quit) { worker_stats[id].handled_packets += num; @@ -284,6 +279,5 @@ handle_work_with_free_mbufs(void *arg) for (i = 0; i < num; i++) rte_pktmbuf_free(buf[i]); - num = rte_distributor_get_pkt(d, - id, buf, buf, num); + num = rte_distributor_get_pkt(d, id, buf, NULL, 0); } worker_stats[id].handled_packets += num; @@ -343,5 +337,5 @@ handle_work_for_shutdown_test(void *arg) struct rte_distributor *d = wp->dist; unsigned int count = 0; - unsigned int num = 0; + unsigned int num; unsigned int total = 0; unsigned int i; @@ -349,5 +343,5 @@ handle_work_for_shutdown_test(void *arg) const unsigned int id = __sync_fetch_and_add(&worker_idx, 1); - num = rte_distributor_get_pkt(d, id, buf, buf, num); + num = rte_distributor_get_pkt(d, id, buf, NULL, 0); /* wait for quit single globally, or for worker zero, wait @@ -358,6 +352,5 @@ handle_work_for_shutdown_test(void *arg) for (i = 0; i < num; i++) rte_pktmbuf_free(buf[i]); - num = rte_distributor_get_pkt(d, - id, buf, buf, num); + num = rte_distributor_get_pkt(d, id, buf, NULL, 0); total += num; } @@ -373,6 +366,5 @@ handle_work_for_shutdown_test(void *arg) usleep(100); - num = rte_distributor_get_pkt(d, - id, buf, buf, num); + num = rte_distributor_get_pkt(d, id, buf, NULL, 0); while (!quit) { @@ -380,5 +372,5 @@ handle_work_for_shutdown_test(void *arg) count += num; rte_pktmbuf_free(pkt); - num = rte_distributor_get_pkt(d, id, buf, buf, num); + num = rte_distributor_get_pkt(d, id, buf, NULL, 0); } returned = rte_distributor_return_pkt(d, -- 2.26.2 --- Diff of the applied patch vs upstream commit (please double-check if non-empty: --- --- - 2020-11-18 16:33:38.352188647 +0000 +++ 0020-distributor-fix-buffer-use-after-free.patch 2020-11-18 16:33:37.926215060 +0000 @@ -1 +1 @@ -From 6bd951b48222caaa10a796057f617cab04f928b0 Mon Sep 17 00:00:00 2001 +From de6e48d0b37ccb04376af977abc27c2833f7364e Mon Sep 17 00:00:00 2001 @@ -5,0 +6,2 @@ +[ upstream commit 6bd951b48222caaa10a796057f617cab04f928b0 ] + @@ -17 +18,0 @@ -Cc: stable@dpdk.org @@ -22 +22,0 @@ - app/test/test_distributor.c | 28 +++++++++--------------- @@ -23,0 +24 @@ + test/test/test_distributor.c | 28 +++++++++--------------- @@ -26,4 +27,22 @@ -diff --git a/app/test/test_distributor.c b/app/test/test_distributor.c -index ba1f81cf8d..52230d2504 100644 ---- a/app/test/test_distributor.c -+++ b/app/test/test_distributor.c +diff --git a/lib/librte_distributor/rte_distributor.c b/lib/librte_distributor/rte_distributor.c +index d1f8b14d2e..5f9dde8494 100644 +--- a/lib/librte_distributor/rte_distributor.c ++++ b/lib/librte_distributor/rte_distributor.c +@@ -44,5 +44,5 @@ rte_distributor_request_pkt_v1705(struct rte_distributor *d, + if (unlikely(d->alg_type == RTE_DIST_ALG_SINGLE)) { + rte_distributor_request_pkt_v20(d->d_v20, +- worker_id, oldpkt[0]); ++ worker_id, count ? oldpkt[0] : NULL); + return; + } +@@ -144,5 +144,5 @@ rte_distributor_get_pkt_v1705(struct rte_distributor *d, + if (return_count <= 1) { + pkts[0] = rte_distributor_get_pkt_v20(d->d_v20, +- worker_id, oldpkt[0]); ++ worker_id, return_count ? oldpkt[0] : NULL); + return (pkts[0]) ? 1 : 0; + } else +diff --git a/test/test/test_distributor.c b/test/test/test_distributor.c +index 6aa4e49e69..ae7eff5a91 100644 +--- a/test/test/test_distributor.c ++++ b/test/test/test_distributor.c @@ -35 +54 @@ - unsigned int id = __atomic_fetch_add(&worker_idx, 1, __ATOMIC_RELAXED); + unsigned int id = __sync_fetch_and_add(&worker_idx, 1); @@ -49 +68 @@ - unsigned int id = __atomic_fetch_add(&worker_idx, 1, __ATOMIC_RELAXED); + unsigned int id = __sync_fetch_and_add(&worker_idx, 1); @@ -72,2 +91,2 @@ -@@ -350,5 +344,5 @@ handle_work_for_shutdown_test(void *arg) - __ATOMIC_RELAXED); +@@ -349,5 +343,5 @@ handle_work_for_shutdown_test(void *arg) + const unsigned int id = __sync_fetch_and_add(&worker_idx, 1); @@ -79 +98 @@ -@@ -359,6 +353,5 @@ handle_work_for_shutdown_test(void *arg) +@@ -358,6 +352,5 @@ handle_work_for_shutdown_test(void *arg) @@ -87 +106 @@ -@@ -374,6 +367,5 @@ handle_work_for_shutdown_test(void *arg) +@@ -373,6 +366,5 @@ handle_work_for_shutdown_test(void *arg) @@ -95 +114 @@ -@@ -381,5 +373,5 @@ handle_work_for_shutdown_test(void *arg) +@@ -380,5 +372,5 @@ handle_work_for_shutdown_test(void *arg) @@ -102,18 +120,0 @@ -diff --git a/lib/librte_distributor/rte_distributor.c b/lib/librte_distributor/rte_distributor.c -index d6d4350a28..93c90cf543 100644 ---- a/lib/librte_distributor/rte_distributor.c -+++ b/lib/librte_distributor/rte_distributor.c -@@ -43,5 +43,5 @@ rte_distributor_request_pkt(struct rte_distributor *d, - if (unlikely(d->alg_type == RTE_DIST_ALG_SINGLE)) { - rte_distributor_request_pkt_single(d->d_single, -- worker_id, oldpkt[0]); -+ worker_id, count ? oldpkt[0] : NULL); - return; - } -@@ -135,5 +135,5 @@ rte_distributor_get_pkt(struct rte_distributor *d, - if (return_count <= 1) { - pkts[0] = rte_distributor_get_pkt_single(d->d_single, -- worker_id, oldpkt[0]); -+ worker_id, return_count ? oldpkt[0] : NULL); - return (pkts[0]) ? 1 : 0; - } else