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 7F71CA0C43; Wed, 16 Jun 2021 18:56:43 +0200 (CEST) Received: from [217.70.189.124] (localhost [127.0.0.1]) by mails.dpdk.org (Postfix) with ESMTP id 621424067A; Wed, 16 Jun 2021 18:56:43 +0200 (CEST) Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) by mails.dpdk.org (Postfix) with ESMTP id BA1C140140 for ; Wed, 16 Jun 2021 18:56:41 +0200 (CEST) Received: by mail-pl1-f174.google.com with SMTP id 11so1410052plk.12 for ; Wed, 16 Jun 2021 09:56:41 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=networkplumber-org.20150623.gappssmtp.com; s=20150623; h=date:from:to:cc:subject:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=ko6aMSQ/bYknVJwbc8jkkf6NZ7uqSVAzUvirXde5XCk=; b=jE7U956YgH2p4u/yK1LOBT2uk2P5gbH4MSGDqFlqwiTEUV3bOZjmsm43ZaootJIzAZ Mxx864ldlLiOZwQVgz4wKDXfrYX0r6+fUgldDOUKGZb7MnaLl/FnJZG+YiV89QG+CIor /m0XYNPTM+QZ4oHGUeuuK8eYcwJPtJ0vm07bU5YtAak4NoXG7FLLwy54GRNp9HMCxIXO /AiqUkcacxHJfBuh0cyLsCVOZGvJikJveHB6KG6GTvHf8Kzmd3HA5RVObMThsIaV2MmW E8V6xCmlV8HxrsqF+dIb0gToq8ZwaELLBuMd5bPMvY/jbWLatnNbp6xeu6IyZl3HZ3+R l8oQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=ko6aMSQ/bYknVJwbc8jkkf6NZ7uqSVAzUvirXde5XCk=; b=rhxbY806yWK7PtKOOwTdvmieiIFn55XrTN+17byoj8XB+p8eAuj5KptGoW7dFtQFt2 Ke5WgKLFv9cxHXR1c5iVX1lU4RLacTfxEcqQe0/y7oS/u/3TDLvytViPqZL+JzVz9HoZ 6GV6rH2YEK5WiJ7HbpK/YglOrSk1pemR8FOfi83uQ6sVQYoozsAF7dNCLqzhVGnt/L8h tAI7RrKOl1SbWuY//K5txcGC82mMkxG6EFMjgVi+QZIw5lgDv0gY6XBrYxvHPAzv/6Sb PVHUrXmtz9HpX53NsGsInsuIXHN6qrf/ArxMZcbotO5NSCRA6r3sOxaTMhHOkaoQrn9Q CsQw== X-Gm-Message-State: AOAM530plHJWDV78l+Guo4YvuIg6ZZcPpWVU8uJQ+JyknT+2tyo9b3AI eH/tQ2lpfHG1DEEDX1GHjk8Lqw== X-Google-Smtp-Source: ABdhPJymqJAVhshYiEpUlC6WAi7proM7vlQQui/QsWi/r+mP9c/CQZsp5qnLhFpqMZYxGkzpy7dYxw== X-Received: by 2002:a17:902:dac3:b029:105:66fc:8ed7 with SMTP id q3-20020a170902dac3b029010566fc8ed7mr482518plx.6.1623862600794; Wed, 16 Jun 2021 09:56:40 -0700 (PDT) Received: from hermes.local (76-14-218-44.or.wavecable.com. [76.14.218.44]) by smtp.gmail.com with ESMTPSA id c14sm2872725pgv.86.2021.06.16.09.56.40 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 16 Jun 2021 09:56:40 -0700 (PDT) Date: Wed, 16 Jun 2021 09:56:32 -0700 From: Stephen Hemminger To: ohilyard@iol.unh.edu Cc: vladimir.medvedkin@intel.com, dev@dpdk.org, david.marchand@redhat.com Message-ID: <20210616095632.0f97eeb3@hermes.local> In-Reply-To: <20210616160730.348523-1-ohilyard@iol.unh.edu> References: <20210616160730.348523-1-ohilyard@iol.unh.edu> MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII Content-Transfer-Encoding: 7bit Subject: Re: [dpdk-dev] [PATCH] 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" On Wed, 16 Jun 2021 12:07:29 -0400 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. > > Signed-off-by: Owen Hilyard > --- > lib/rib/rte_rib6.c | 22 ++++++++++++++-------- > 1 file changed, 14 insertions(+), 8 deletions(-) > > diff --git a/lib/rib/rte_rib6.c b/lib/rib/rte_rib6.c > index f6c55ee45..2de50449d 100644 > --- a/lib/rib/rte_rib6.c > +++ b/lib/rib/rte_rib6.c > @@ -79,14 +79,20 @@ 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; > + int index, msk; > + /* depth & 127 clamps depth to values that will not Please put blank line after declarations for clarity. Since index and mask are not signed values, please make them unsigned. Better yet, make them sized to the appropriate number of bits. > + * 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 & 127) >> 3; > + /* > + * 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 *