From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 9BF041B4D4; Fri, 5 Apr 2019 14:53:27 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 0EAAB21B24; Fri, 5 Apr 2019 08:53:27 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Fri, 05 Apr 2019 08:53:27 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=OYmOSDLpu4CLgOALO/0mQNb8VTDlcWSBn+C9vLaPh+w=; b=S/gNvikrcd5k qunmFgj9mkGnZOA9ClDdTWvzFJCMIMVyYySs6vILK84YmHmSbStg+5im1d6nOP7H KLFjmWpThznPKF8j8tBk8N65ZS+pB7+3FxPSErSJ8/aX/U1vhDzMdLUHiISsysJk e8/DnQ/J5i3suNA4kld/jCoeKUGRWLo= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=OYmOSDLpu4CLgOALO/0mQNb8VTDlcWSBn+C9vLaPh +w=; b=tJoHbwRBn5KNBqpMzk0n0lXRD+y19BLXdKMlAN0uDhUr/wWQ20yfcDQR2 AxLgXAXGx+1AnBtdXJo97yVDs3muwO2jJDa6ROdCX2Tt98bn6puXlEkBZNdkVcft by31OxSaDdg2rmO9jKXNmAh0gum5XeIA+TOHmNmVve9GpHNCFgALzavUAyHuQ7av m+gLifjRai+Mxr1Jm8G9mxFLioF89e9VRxSG45Mm7icHAnFPBvKm9uxpxNsWQYSk 1VCjhFYd8us3cbmYu0oZCbAMbAS6ozVV8Z3AI8nD0KUcp3HDkLdJR5+GC5N2M934 w0JHaT+Er0q30Goq5iIM/KVo5Zg1g== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduuddrtdejgdehgeculddtuddrgedutddrtddtmd cutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdp uffrtefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivg hnthhsucdlqddutddtmdenucfjughrpefhvffufffkjghfggfgtgesthfuredttddtvden ucfhrhhomhepvfhhohhmrghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrg hlohhnrdhnvghtqeenucfkphepjeejrddufeegrddvtdefrddukeegnecurfgrrhgrmhep mhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtnecuvehluhhsth gvrhfuihiivgeptd X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 65669E452B; Fri, 5 Apr 2019 08:53:25 -0400 (EDT) From: Thomas Monjalon To: "Chaitanya Babu, TalluriX" Cc: dev@dpdk.org, Ferruh Yigit , "Richardson, Bruce" , "Pattan, Reshma" , "Parthasarathy, JananeeX M" , "Dumitrescu, Cristian" , "stable@dpdk.org" Date: Fri, 05 Apr 2019 14:53:22 +0200 Message-ID: <2188499.5WO9SvJthX@xps> In-Reply-To: References: <1550136631-32415-1-git-send-email-tallurix.chaitanya.babu@intel.com> <761FB0F2AB727F4FA9CE98D18810B0151B1F18AA@BGSMSX103.gar.corp.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="us-ascii" Subject: Re: [dpdk-dev] [PATCH v2] lib/cfgfile: replace strcat with strlcat X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Fri, 05 Apr 2019 12:53:28 -0000 27/03/2019 12:37, Ferruh Yigit: > On 3/26/2019 10:04 AM, Chaitanya Babu, TalluriX wrote: > > From: Yigit, Ferruh > >> On 3/8/2019 2:02 PM, Bruce Richardson wrote: > >>> On Fri, Mar 08, 2019 at 12:45:50PM +0000, Chaitanya Babu Talluri wrote: > >>>> Replace strcat with strlcat to avoid buffer overflow. > >>>> > >>>> Fixes: a6a47ac9c2 ("cfgfile: rework load function") > >>>> Cc: stable@dpdk.org > >>>> > >>>> Signed-off-by: Chaitanya Babu Talluri > >>>> > >>>> --- > >>>> @@ -224,10 +225,11 @@ rte_cfgfile_load_with_params(const char > >> *filename, int flags, > >>>> _strip(split[1], strlen(split[1])); > >>>> char *end = memchr(split[1], '\\', strlen(split[1])); > >>>> > >>>> + size_t split_len = strlen(split[1]) + 1; > >>>> while (end != NULL) { > >>>> if (*(end+1) == params->comment_character) > >> { > >>>> *end = '\0'; > >>>> - strcat(split[1], end+1); > >>>> + strlcat(split[1], end+1, split_len); > >>> > >>> I don't think this will do what you want. Remember that strlcat takes > >>> the total length of the buffer, which means that if split_len is set > >>> to the current length (as you do before the while statement), then > >>> passing that as the length parameter will cause strlcat to do nothing, > >>> since it sees the buffer as already full. > >> > >> The logic doesn't lengthen the 'split[1]' content, indeed it reduces the initial > >> size although it uses string concatenation, that is why it should be OK to use > >> 'split_len' here. > >> > >> What code does is, it finds specific char in 'split' buffer and removes it by > >> shifting remaining chars one byte to the left. So it shouldn't pass the initial size > >> of the buffer. > >> > >> There is a overlapping strings concern, which 'strcat' & 'strlcat' don't support, > >> but I guess it is OK here since we are sure that strings are separated by a > >> NULL, so where a char read and written should be different although overall > >> dst and src buffers overlap. > > > > Yes, although the same string is manipulated the split string (*end = '\0') is separated with NULL. > > Strlcat works fine here and expected concatenation is happening. > > If there are no further comments request for ACK please. > > Acked-by: Ferruh Yigit Applied, thanks From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by dpdk.space (Postfix) with ESMTP id A1FFFA0679 for ; Fri, 5 Apr 2019 14:53:32 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D7E621B4DE; Fri, 5 Apr 2019 14:53:29 +0200 (CEST) Received: from out1-smtp.messagingengine.com (out1-smtp.messagingengine.com [66.111.4.25]) by dpdk.org (Postfix) with ESMTP id 9BF041B4D4; Fri, 5 Apr 2019 14:53:27 +0200 (CEST) Received: from compute1.internal (compute1.nyi.internal [10.202.2.41]) by mailout.nyi.internal (Postfix) with ESMTP id 0EAAB21B24; Fri, 5 Apr 2019 08:53:27 -0400 (EDT) Received: from mailfrontend1 ([10.202.2.162]) by compute1.internal (MEProxy); Fri, 05 Apr 2019 08:53:27 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=monjalon.net; h= from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding:content-type; s=mesmtp; bh=OYmOSDLpu4CLgOALO/0mQNb8VTDlcWSBn+C9vLaPh+w=; b=S/gNvikrcd5k qunmFgj9mkGnZOA9ClDdTWvzFJCMIMVyYySs6vILK84YmHmSbStg+5im1d6nOP7H KLFjmWpThznPKF8j8tBk8N65ZS+pB7+3FxPSErSJ8/aX/U1vhDzMdLUHiISsysJk e8/DnQ/J5i3suNA4kld/jCoeKUGRWLo= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d= messagingengine.com; h=cc:content-transfer-encoding:content-type :date:from:in-reply-to:message-id:mime-version:references :subject:to:x-me-proxy:x-me-proxy:x-me-sender:x-me-sender :x-sasl-enc; s=fm2; bh=OYmOSDLpu4CLgOALO/0mQNb8VTDlcWSBn+C9vLaPh +w=; b=tJoHbwRBn5KNBqpMzk0n0lXRD+y19BLXdKMlAN0uDhUr/wWQ20yfcDQR2 AxLgXAXGx+1AnBtdXJo97yVDs3muwO2jJDa6ROdCX2Tt98bn6puXlEkBZNdkVcft by31OxSaDdg2rmO9jKXNmAh0gum5XeIA+TOHmNmVve9GpHNCFgALzavUAyHuQ7av m+gLifjRai+Mxr1Jm8G9mxFLioF89e9VRxSG45Mm7icHAnFPBvKm9uxpxNsWQYSk 1VCjhFYd8us3cbmYu0oZCbAMbAS6ozVV8Z3AI8nD0KUcp3HDkLdJR5+GC5N2M934 w0JHaT+Er0q30Goq5iIM/KVo5Zg1g== X-ME-Sender: X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduuddrtdejgdehgeculddtuddrgedutddrtddtmd cutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdp uffrtefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivg hnthhsucdlqddutddtmdenucfjughrpefhvffufffkjghfggfgtgesthfuredttddtvden ucfhrhhomhepvfhhohhmrghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrg hlohhnrdhnvghtqeenucfkphepjeejrddufeegrddvtdefrddukeegnecurfgrrhgrmhep mhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtnecuvehluhhsth gvrhfuihiivgeptd X-ME-Proxy: Received: from xps.localnet (184.203.134.77.rev.sfr.net [77.134.203.184]) by mail.messagingengine.com (Postfix) with ESMTPA id 65669E452B; Fri, 5 Apr 2019 08:53:25 -0400 (EDT) From: Thomas Monjalon To: "Chaitanya Babu, TalluriX" Cc: dev@dpdk.org, Ferruh Yigit , "Richardson, Bruce" , "Pattan, Reshma" , "Parthasarathy, JananeeX M" , "Dumitrescu, Cristian" , "stable@dpdk.org" Date: Fri, 05 Apr 2019 14:53:22 +0200 Message-ID: <2188499.5WO9SvJthX@xps> In-Reply-To: References: <1550136631-32415-1-git-send-email-tallurix.chaitanya.babu@intel.com> <761FB0F2AB727F4FA9CE98D18810B0151B1F18AA@BGSMSX103.gar.corp.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 7Bit Content-Type: text/plain; charset="UTF-8" Subject: Re: [dpdk-dev] [PATCH v2] lib/cfgfile: replace strcat with strlcat X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 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" Message-ID: <20190405125322.LlewruB6IZwuMbMreA4WrJpA6IYFWkc3_qEC3Swh-kw@z> 27/03/2019 12:37, Ferruh Yigit: > On 3/26/2019 10:04 AM, Chaitanya Babu, TalluriX wrote: > > From: Yigit, Ferruh > >> On 3/8/2019 2:02 PM, Bruce Richardson wrote: > >>> On Fri, Mar 08, 2019 at 12:45:50PM +0000, Chaitanya Babu Talluri wrote: > >>>> Replace strcat with strlcat to avoid buffer overflow. > >>>> > >>>> Fixes: a6a47ac9c2 ("cfgfile: rework load function") > >>>> Cc: stable@dpdk.org > >>>> > >>>> Signed-off-by: Chaitanya Babu Talluri > >>>> > >>>> --- > >>>> @@ -224,10 +225,11 @@ rte_cfgfile_load_with_params(const char > >> *filename, int flags, > >>>> _strip(split[1], strlen(split[1])); > >>>> char *end = memchr(split[1], '\\', strlen(split[1])); > >>>> > >>>> + size_t split_len = strlen(split[1]) + 1; > >>>> while (end != NULL) { > >>>> if (*(end+1) == params->comment_character) > >> { > >>>> *end = '\0'; > >>>> - strcat(split[1], end+1); > >>>> + strlcat(split[1], end+1, split_len); > >>> > >>> I don't think this will do what you want. Remember that strlcat takes > >>> the total length of the buffer, which means that if split_len is set > >>> to the current length (as you do before the while statement), then > >>> passing that as the length parameter will cause strlcat to do nothing, > >>> since it sees the buffer as already full. > >> > >> The logic doesn't lengthen the 'split[1]' content, indeed it reduces the initial > >> size although it uses string concatenation, that is why it should be OK to use > >> 'split_len' here. > >> > >> What code does is, it finds specific char in 'split' buffer and removes it by > >> shifting remaining chars one byte to the left. So it shouldn't pass the initial size > >> of the buffer. > >> > >> There is a overlapping strings concern, which 'strcat' & 'strlcat' don't support, > >> but I guess it is OK here since we are sure that strings are separated by a > >> NULL, so where a char read and written should be different although overall > >> dst and src buffers overlap. > > > > Yes, although the same string is manipulated the split string (*end = '\0') is separated with NULL. > > Strlcat works fine here and expected concatenation is happening. > > If there are no further comments request for ACK please. > > Acked-by: Ferruh Yigit Applied, thanks