From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp-fw-2101.amazon.com (smtp-fw-2101.amazon.com [72.21.196.25]) by dpdk.org (Postfix) with ESMTP id C18032BA2 for ; Thu, 8 Jun 2017 21:07:53 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=amazon.com; i=@amazon.com; q=dns/txt; s=amazon201209; t=1496948873; x=1528484873; h=from:to:subject:date:message-id:references:in-reply-to: content-transfer-encoding:mime-version; bh=A2yRFE0rknGUwWwJCTgScLzXtut7Oce6pYr+7EXj9yw=; b=dA8s9c6FIcpHD8PP6p/9cXNHXd0FvRGybvP/Y3LhO61iVeoaLWbI1DRu 9OAGO+rjHMmy+mmb7sle9qaZfbgSR6o38wDVcQwXyO3rI3gv0bzfbFqoN TXmnzdWryuxDE6BMK0h1kTWCDd04QmHI42T5sZtf6117RkTEELU5S1pmJ Q=; X-IronPort-AV: E=Sophos;i="5.39,315,1493683200"; d="scan'208";a="652989408" Received: from iad6-co-svc-p1-lb1-vlan2.amazon.com (HELO email-inbound-relay-71001.iad55.amazon.com) ([10.124.125.2]) by smtp-border-fw-out-2101.iad2.amazon.com with ESMTP/TLS/DHE-RSA-AES256-SHA; 08 Jun 2017 19:07:45 +0000 Received: from EX13MTAUWA001.ant.amazon.com (iad55-ws-svc-p15-lb9-vlan3.iad.amazon.com [10.40.159.166]) by email-inbound-relay-71001.iad55.amazon.com (8.14.7/8.14.7) with ESMTP id v58J7f0G024286 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=FAIL); Thu, 8 Jun 2017 19:07:43 GMT Received: from EX13d09UWA003.ant.amazon.com (10.43.160.227) by EX13MTAUWA001.ant.amazon.com (10.43.160.58) with Microsoft SMTP Server (TLS) id 15.0.1104.5; Thu, 8 Jun 2017 19:07:43 +0000 Received: from EX13d09UWA003.ant.amazon.com (10.43.160.227) by EX13d09UWA003.ant.amazon.com (10.43.160.227) with Microsoft SMTP Server (TLS) id 15.0.1104.5; Thu, 8 Jun 2017 19:07:42 +0000 Received: from EX13d09UWA003.ant.amazon.com ([10.43.160.227]) by EX13d09UWA003.ant.amazon.com ([10.43.160.227]) with mapi id 15.00.1104.000; Thu, 8 Jun 2017 19:07:42 +0000 From: "Lavigne, Jamie" To: Sergio Gonzalez Monroy , "dev@dpdk.org" Thread-Topic: [dpdk-dev] [PATCH v2] Correctly handle malloc_elem resize with padding Thread-Index: AQHS2aM8PFKPpBSUMkCfNd+QQSh4CaIX7E+AgANzhxg= Date: Thu, 8 Jun 2017 19:07:42 +0000 Message-ID: <1496948862726.3852@amazon.com> References: <1496189340-27813-1-git-send-email-lavignen@amazon.com> <1496189818-2307-1-git-send-email-lavignen@amazon.com>, <3b80fa21-3d3e-73ae-235b-7581db5e8670@intel.com> In-Reply-To: <3b80fa21-3d3e-73ae-235b-7581db5e8670@intel.com> Accept-Language: en-US Content-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-ms-exchange-transport-fromentityheader: Hosted x-originating-ip: [10.43.161.176] Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable MIME-Version: 1.0 Precedence: Bulk Subject: Re: [dpdk-dev] [PATCH v2] Correctly handle malloc_elem resize with padding X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 08 Jun 2017 19:07:54 -0000 Hi Sergio,=0A= =0A= > Hi Jamie, =0A= > =0A= > On 31/05/2017 01:16, Jamie Lavigne wrote: =0A= > > Currently when a malloc_elem is split after resizing, any padding =0A= > > present in the elem is ignored. This causes the resized elem to be too= =0A= > > small when padding is present, and user data can overwrite the beginnin= g =0A= > > of the following malloc_elem. =0A= > > =0A= > > Solve this by including the size of the padding when computing where to= =0A= > > split the malloc_elem. =0A= > =0A= > Nice catch! =0A= > =0A= > Could you please rework commit format a bit: =0A= > - Add 'mem:' as prefix in your patch title =0A= > - I would mention in the title that this is a fix =0A= > - Provide 'Fixes' line in commit message =0A= =0A= Updated.=0A= =0A= > =0A= > > Signed-off-by: Jamie Lavigne =0A= > > --- =0A= > > lib/librte_eal/common/malloc_elem.c | 6 ++++-- =0A= > > 1 file changed, 4 insertions(+), 2 deletions(-) =0A= > > =0A= > > diff --git a/lib/librte_eal/common/malloc_elem.c b/lib/librte_eal/commo= n/malloc_elem.c =0A= > > index 42568e1..8766fa8 100644 =0A= > > --- a/lib/librte_eal/common/malloc_elem.c =0A= > > +++ b/lib/librte_eal/common/malloc_elem.c =0A= > > @@ -333,9 +333,11 @@ malloc_elem_resize(struct malloc_elem *elem, size_= t size) =0A= > > elem_free_list_remove(next); =0A= > > join_elem(elem, next); =0A= > > =0A= > > - if (elem->size - new_size >=3D MIN_DATA_SIZE + MALLOC_ELEM_OVERHE= AD){ =0A= > > + const size_t new_total_size =3D new_size + elem->pad; =0A= > > + =0A= > > + if (elem->size - new_total_size >=3D MIN_DATA_SIZE + MALLOC_ELEM_= OVERHEAD) { =0A= > > /* now we have a big block together. Lets cut it down a b= it, by splitting */ =0A= > > - struct malloc_elem *split_pt =3D RTE_PTR_ADD(elem, new_si= ze); =0A= > > + struct malloc_elem *split_pt =3D RTE_PTR_ADD(elem, new_to= tal_size); =0A= > > split_pt =3D RTE_PTR_ALIGN_CEIL(split_pt, RTE_CACHE_LINE_= SIZE); =0A= > > split_elem(elem, split_pt); =0A= > > malloc_elem_free_list_insert(split_pt); =0A= > =0A= > This indeed fixes the issue you have mentioned. I was thinking of the =0A= > following fix instead: =0A= > - Add elem->pad to new_size =0A= > - Remove current_size var and instead use elem->size =0A= > =0A= > I think those changes should have the same result while removing a =0A= > couple of vars from the function, which I hope would be easier to read. = =0A= > =0A= > What do you think? =0A= =0A= I like this. It looks equivalent to my solution, but simpler. I will post= an updated patch.=0A= =0A= Jamie=0A=