From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mails.dpdk.org (mails.dpdk.org [217.70.189.124]) by inbox.dpdk.org (Postfix) with ESMTP id 01A44A04FD; Mon, 1 Aug 2022 15:14:01 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id E502842C91; Mon, 1 Aug 2022 15:14:00 +0200 (CEST) Received: from mail-qk1-f179.google.com (mail-qk1-f179.google.com [209.85.222.179]) by mails.dpdk.org (Postfix) with ESMTP id 32FF542C91 for ; Mon, 1 Aug 2022 15:13:59 +0200 (CEST) Received: by mail-qk1-f179.google.com with SMTP id n2so8273780qkk.8; Mon, 01 Aug 2022 06:13:59 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:from:to:cc; bh=3Px05MIeZCy/SMatwkbbVRfkFQZHQjKA/J+hXGmMmSM=; b=aL82UhfrIU7igf4/ruCjzDHh7r870cUXitclWZrmhOkElL50bF6BicYJ7zKEqpxl3R Dl+7pg2w8dD0qIjZKVs5ijq00S/ZgXEgwTuLNvXLFNHjQx3K4++eBjA4YzmN/4UZ+g/i T44UV7VW29L47UN4+o/5pAr7Y2YSUMMlTmyyEaZrS0Z32vULi02AqZ/TGPZj93FtDpJU GAbbikT5JzxEZM7JvEU+dDfC+kNGL1zgF+Muhd1/p/ufpaIGcWq8/PHeR0jS3NQ2XbmH zyzLaQtEr6eQ3v8ZF864XSxgD4rHOn1LjmbRpjWx3+1ryk32+tDou6vsARYw7IjUFxxm y91A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc; bh=3Px05MIeZCy/SMatwkbbVRfkFQZHQjKA/J+hXGmMmSM=; b=5yXeZugIBhn80NdXrJAfqO7zLO6RhQ+fgpi+LF9tcs5q2mqmf0wgjxKCR52tu+rdl8 6voiS4e+I2PCZzVmsPVdESbqFpCi4Cf76piCRF/+BS302k5cYRIKWecgrjg69DzONt4P T7xuoV9k0bqjiFhRxloMd/QLPR0hWUirF6mV7WuToirxENn2M3NIa2LM91yAvQ9mEWZP 1EnnPwvxhOTCKHgtVTW0irh2qa6ZIGnyQH/E5YDmTszRfGAg6ao6kgS31nNGiRrgLUkK 9FDEk+WlDQJ+aFkfHNiZ35P3Dnu6M96OsnpMrXERLpun54MORyss4uVnjMy08wCPdHj5 QWpA== X-Gm-Message-State: AJIora+aDdWQbOqtWHVjVo1Dv12WNswsqwMUmFXxpJKxAxL4X63J3Le9 VpeOwxaBXBXLNbR4fwb64jW6sCbLSRMAi9SVI4s= X-Google-Smtp-Source: AGRyM1uXMZP8M2AxOo15Tgp0u7LiQiWgk8PDHD2mKkUy5Oj3HD6mjj1fe6VDLNOpXH5UXYag3J2gF/JMcwjMOtd512c= X-Received: by 2002:a05:620a:424c:b0:6aa:cdf8:f6f3 with SMTP id w12-20020a05620a424c00b006aacdf8f6f3mr11668262qko.26.1659359638499; Mon, 01 Aug 2022 06:13:58 -0700 (PDT) MIME-Version: 1.0 References: <20220727023924.2066465-1-zhirun.yan@intel.com> In-Reply-To: <20220727023924.2066465-1-zhirun.yan@intel.com> From: Jerin Jacob Date: Mon, 1 Aug 2022 18:43:32 +0530 Message-ID: Subject: Re: [PATCH v1] graph: fix out of bounds access when re-allocate node objs To: Zhirun Yan Cc: dpdk-dev , Jerin Jacob , Kiran Kumar K , Liang@dpdk.org, Cunming Content-Type: text/plain; charset="UTF-8" X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org On Wed, Jul 27, 2022 at 8:10 AM Zhirun Yan wrote: > > For __rte_node_enqueue_prologue(), If the number of objs is more than > the node->size * 2, the extra objs will write out of bounds memory. > It should use __rte_node_stream_alloc_size() to request enough memory. > > And for rte_node_next_stream_put(), it will re-allocate a small size, > when the node free space is small and new objs is less than the current > node->size. Some objs pointers behind new size may be lost. And it will > cause memory leak. It should request enough size of memory, containing > the original objs and new objs at least. > > Fixes: 40d4f51403ec ("graph: implement fastpath routines") > > Signed-off-by: Zhirun Yan > Signed-off-by: Liang, Cunming > --- > lib/graph/rte_graph_worker.h | 14 ++++++++++---- > 1 file changed, 10 insertions(+), 4 deletions(-) > > diff --git a/lib/graph/rte_graph_worker.h b/lib/graph/rte_graph_worker.h > index 0c0b9c095a..b7d145c3cb 100644 > --- a/lib/graph/rte_graph_worker.h > +++ b/lib/graph/rte_graph_worker.h > @@ -218,13 +218,16 @@ static __rte_always_inline void > __rte_node_enqueue_prologue(struct rte_graph *graph, struct rte_node *node, > const uint16_t idx, const uint16_t space) > { > + uint32_t req_size; > > /* Add to the pending stream list if the node is new */ > if (idx == 0) > __rte_node_enqueue_tail_update(graph, node); > > - if (unlikely(node->size < (idx + space))) > - __rte_node_stream_alloc(graph, node); > + if (unlikely(node->size < (idx + space))) { > + req_size = rte_align32pow2(node->size + space); > + __rte_node_stream_alloc_size(graph, node, req_size); > + } Change looks good to me. Please have an inline function to avoid code duplication(Same change in rte_node_next_stream_get()) With above change: Acked-by: Jerin Jacob > } > > /** > @@ -430,9 +433,12 @@ rte_node_next_stream_get(struct rte_graph *graph, struct rte_node *node, > node = __rte_node_next_node_get(node, next); > const uint16_t idx = node->idx; > uint16_t free_space = node->size - idx; > + uint32_t req_size; > > - if (unlikely(free_space < nb_objs)) > - __rte_node_stream_alloc_size(graph, node, nb_objs); > + if (unlikely(free_space < nb_objs)) { > + req_size = rte_align32pow2(node->size + nb_objs); > + __rte_node_stream_alloc_size(graph, node, req_size); > + } > > return &node->objs[idx]; > } > -- > 2.25.1 >