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 364C4A034F; Thu, 24 Jun 2021 11:01:26 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 1BB5940141; Thu, 24 Jun 2021 11:01:26 +0200 (CEST) Received: from mga06.intel.com (mga06.intel.com [134.134.136.31]) by mails.dpdk.org (Postfix) with ESMTP id 0676F4003C for ; Thu, 24 Jun 2021 11:01:24 +0200 (CEST) IronPort-SDR: Cd1BIKRhOsXYhaZejRN6fo/I4EcGeDRBZ17kWOtZMdR/iIQoV2yO3kwl4BQrTK9TDAfcB9Zwla LamcW4H2pcYg== X-IronPort-AV: E=McAfee;i="6200,9189,10024"; a="268564244" X-IronPort-AV: E=Sophos;i="5.83,296,1616482800"; d="scan'208";a="268564244" Received: from orsmga002.jf.intel.com ([10.7.209.21]) by orsmga104.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Jun 2021 02:01:23 -0700 IronPort-SDR: hu1dxh65yi56LI4wLbdwqnPaykIIvH71qrmZNOF3g7Im2Q+vGddkx89dvyIQQxVWZeWDAECW8l nYmdjpDC7Fqg== X-IronPort-AV: E=Sophos;i="5.83,296,1616482800"; d="scan'208";a="424005635" Received: from vmedvedk-mobl.ger.corp.intel.com (HELO [10.252.13.51]) ([10.252.13.51]) by orsmga002-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 24 Jun 2021 02:01:21 -0700 To: ohilyard@iol.unh.edu Cc: dev@dpdk.org, stephen@networkplumber.org, david.marchand@redhat.com References: <20210616181833.356159-1-ohilyard@iol.unh.edu> <20210621132834.21673-1-ohilyard@iol.unh.edu> From: "Medvedkin, Vladimir" Message-ID: Date: Thu, 24 Jun 2021 12:01:16 +0300 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:78.0) Gecko/20100101 Thunderbird/78.11.0 MIME-Version: 1.0 In-Reply-To: <20210621132834.21673-1-ohilyard@iol.unh.edu> Content-Type: text/plain; charset=utf-8; format=flowed Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH v3] lib/rte_rib6: fix stack buffer overflow 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 Sender: "dev" Acked-by: Vladimir Medvedkin On 21/06/2021 16:28, ohilyard@iol.unh.edu wrote: > From: Owen Hilyard > > ASAN found a stack buffer overflow in lib/rib/rte_rib6.c:get_dir. > The fix for the stack buffer overflow was to make sure depth > was always < 128, since when depth = 128 it caused the index > into the ip address to be 16, which read off the end of the array. > > While trying to solve the buffer overflow, I noticed that a few > changes could be made to remove the for loop entirely. > > Fixes: f7e861e21c ("rib: support IPv6") > > Signed-off-by: Owen Hilyard > --- > lib/rib/rte_rib6.c | 29 +++++++++++++++++++++-------- > 1 file changed, 21 insertions(+), 8 deletions(-) > > diff --git a/lib/rib/rte_rib6.c b/lib/rib/rte_rib6.c > index f6c55ee45..96424e9c9 100644 > --- a/lib/rib/rte_rib6.c > +++ b/lib/rib/rte_rib6.c > @@ -79,20 +79,33 @@ is_covered(const uint8_t ip1[RTE_RIB6_IPV6_ADDR_SIZE], > static inline int > get_dir(const uint8_t ip[RTE_RIB6_IPV6_ADDR_SIZE], uint8_t depth) > { > - int i = 0; > - uint8_t p_depth, msk; > - > - for (p_depth = depth; p_depth >= 8; p_depth -= 8) > - i++; > - > - msk = 1 << (7 - p_depth); > - return (ip[i] & msk) != 0; > + uint8_t index, msk; > + > + /* > + * depth & 127 clamps depth to values that will not > + * read off the end of ip. > + * depth is the number of bits deep into ip to traverse, and > + * is incremented in blocks of 8 (1 byte). This means the last > + * 3 bits are irrelevant to what the index of ip should be. > + */ > + index = (depth & (UINT8_MAX - 1)) / CHAR_BIT; > + > + /* > + * msk is the bitmask used to extract the bit used to decide the > + * direction of the next step of the binary search. > + */ > + msk = 1 << (7 - (depth & 7)); > + > + return (ip[index] & msk) != 0; > } > > static inline struct rte_rib6_node * > get_nxt_node(struct rte_rib6_node *node, > const uint8_t ip[RTE_RIB6_IPV6_ADDR_SIZE]) > { > + if (node->depth == RIB6_MAXDEPTH) > + return NULL; > + > return (get_dir(ip, node->depth)) ? node->right : node->left; > } > > -- Regards, Vladimir