From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <thomas@monjalon.net>
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: <xms:xk-nXOWT1tXXsPydlfF8IXPN9fCrTIMnPurc1WT0nI6ShAmyFkEPyg>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduuddrtdejgdehgeculddtuddrgedutddrtddtmd
cutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdp
uffrtefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivg
hnthhsucdlqddutddtmdenucfjughrpefhvffufffkjghfggfgtgesthfuredttddtvden
ucfhrhhomhepvfhhohhmrghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrg
hlohhnrdhnvghtqeenucfkphepjeejrddufeegrddvtdefrddukeegnecurfgrrhgrmhep
mhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtnecuvehluhhsth
gvrhfuihiivgeptd
X-ME-Proxy: <xmx:xk-nXISdbMuZWi46Uy-L15dsbjWi8L2ew9H2tRT01BdDts0YyXW-Cg>
<xmx:xk-nXEKT5jP3KtRoXQKAzDs98xpf-1LBscpBTJSZHgxDCx-ibRm_gQ>
<xmx:xk-nXB24vbnKMPvsOyPRsQprovzW1W3a38akiVB_kq-SanbR9cV_Tw>
<xmx:x0-nXGapayg5W4w6HW-9FWarwZzMyvIUbFzngRj0vVyPOabdk2W7hQ>
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 <thomas@monjalon.net>
To: "Chaitanya Babu, TalluriX" <tallurix.chaitanya.babu@intel.com>
Cc: dev@dpdk.org, Ferruh Yigit <ferruh.yigit@intel.com>, "Richardson,
Bruce" <bruce.richardson@intel.com>, "Pattan,
Reshma" <reshma.pattan@intel.com>, "Parthasarathy,
JananeeX M" <jananeex.m.parthasarathy@intel.com>, "Dumitrescu,
Cristian" <cristian.dumitrescu@intel.com>, "stable@dpdk.org" <stable@dpdk.org>
Date: Fri, 05 Apr 2019 14:53:22 +0200
Message-ID: <2188499.5WO9SvJthX@xps>
In-Reply-To: <d5f01da4-b9a7-67f1-860a-dcd7c354da40@intel.com>
References: <1550136631-32415-1-git-send-email-tallurix.chaitanya.babu@intel.com>
<761FB0F2AB727F4FA9CE98D18810B0151B1F18AA@BGSMSX103.gar.corp.intel.com>
<d5f01da4-b9a7-67f1-860a-dcd7c354da40@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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=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
> >>>> <tallurix.chaitanya.babu@intel.com>
> >>>> ---
> >>>> @@ -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 <ferruh.yigit@intel.com>
Applied, thanks
From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <dev-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by dpdk.space (Postfix) with ESMTP id A1FFFA0679
for <public@inbox.dpdk.org>; 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: <xms:xk-nXOWT1tXXsPydlfF8IXPN9fCrTIMnPurc1WT0nI6ShAmyFkEPyg>
X-ME-Proxy-Cause: gggruggvucftvghtrhhoucdtuddrgeduuddrtdejgdehgeculddtuddrgedutddrtddtmd
cutefuodetggdotefrodftvfcurfhrohhfihhlvgemucfhrghsthforghilhdpqfgfvfdp
uffrtefokffrpgfnqfghnecuuegrihhlohhuthemuceftddtnecusecvtfgvtghiphhivg
hnthhsucdlqddutddtmdenucfjughrpefhvffufffkjghfggfgtgesthfuredttddtvden
ucfhrhhomhepvfhhohhmrghsucfoohhnjhgrlhhonhcuoehthhhomhgrshesmhhonhhjrg
hlohhnrdhnvghtqeenucfkphepjeejrddufeegrddvtdefrddukeegnecurfgrrhgrmhep
mhgrihhlfhhrohhmpehthhhomhgrshesmhhonhhjrghlohhnrdhnvghtnecuvehluhhsth
gvrhfuihiivgeptd
X-ME-Proxy: <xmx:xk-nXISdbMuZWi46Uy-L15dsbjWi8L2ew9H2tRT01BdDts0YyXW-Cg>
<xmx:xk-nXEKT5jP3KtRoXQKAzDs98xpf-1LBscpBTJSZHgxDCx-ibRm_gQ>
<xmx:xk-nXB24vbnKMPvsOyPRsQprovzW1W3a38akiVB_kq-SanbR9cV_Tw>
<xmx:x0-nXGapayg5W4w6HW-9FWarwZzMyvIUbFzngRj0vVyPOabdk2W7hQ>
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 <thomas@monjalon.net>
To: "Chaitanya Babu, TalluriX" <tallurix.chaitanya.babu@intel.com>
Cc: dev@dpdk.org, Ferruh Yigit <ferruh.yigit@intel.com>, "Richardson,
Bruce" <bruce.richardson@intel.com>, "Pattan,
Reshma" <reshma.pattan@intel.com>, "Parthasarathy,
JananeeX M" <jananeex.m.parthasarathy@intel.com>, "Dumitrescu,
Cristian" <cristian.dumitrescu@intel.com>, "stable@dpdk.org" <stable@dpdk.org>
Date: Fri, 05 Apr 2019 14:53:22 +0200
Message-ID: <2188499.5WO9SvJthX@xps>
In-Reply-To: <d5f01da4-b9a7-67f1-860a-dcd7c354da40@intel.com>
References: <1550136631-32415-1-git-send-email-tallurix.chaitanya.babu@intel.com>
<761FB0F2AB727F4FA9CE98D18810B0151B1F18AA@BGSMSX103.gar.corp.intel.com>
<d5f01da4-b9a7-67f1-860a-dcd7c354da40@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 <dev.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/dev>,
<mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/dev>,
<mailto:dev-request@dpdk.org?subject=subscribe>
Errors-To: dev-bounces@dpdk.org
Sender: "dev" <dev-bounces@dpdk.org>
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
> >>>> <tallurix.chaitanya.babu@intel.com>
> >>>> ---
> >>>> @@ -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 <ferruh.yigit@intel.com>
Applied, thanks