From mboxrd@z Thu Jan 1 00:00:00 1970
Return-Path: <stable-bounces@dpdk.org>
Received: from dpdk.org (dpdk.org [92.243.14.124])
by inbox.dpdk.org (Postfix) with ESMTP id 3F268A0613
for <public@inbox.dpdk.org>; Tue, 30 Jul 2019 16:48:44 +0200 (CEST)
Received: from [92.243.14.124] (localhost [127.0.0.1])
by dpdk.org (Postfix) with ESMTP id 6675E1C127;
Tue, 30 Jul 2019 16:48:42 +0200 (CEST)
Received: from mga17.intel.com (mga17.intel.com [192.55.52.151])
by dpdk.org (Postfix) with ESMTP id A1FCF1B997;
Tue, 30 Jul 2019 16:48:35 +0200 (CEST)
X-Amp-Result: SKIPPED(no attachment in message)
X-Amp-File-Uploaded: False
Received: from orsmga007.jf.intel.com ([10.7.209.58])
by fmsmga107.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384;
30 Jul 2019 07:48:34 -0700
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.64,327,1559545200"; d="scan'208";a="162599880"
Received: from fyigit-mobl.ger.corp.intel.com (HELO [10.237.221.10])
([10.237.221.10])
by orsmga007.jf.intel.com with ESMTP; 30 Jul 2019 07:48:31 -0700
To: Aaron Conole <aconole@redhat.com>,
David Marchand <david.marchand@redhat.com>
Cc: Bernard Iremonger <bernard.iremonger@intel.com>, dev <dev@dpdk.org>,
dpdk stable <stable@dpdk.org>, Thomas Monjalon <thomas@monjalon.net>,
"Singh, Jasvinder" <jasvinder.singh@intel.com>,
Flavia Musatescu <flavia.musatescu@intel.com>,
Adrien Mazarguil <adrien.mazarguil@6wind.com>
References: <1562670596-27129-1-git-send-email-bernard.iremonger@intel.com>
<10372251.KTS5ePcUbj@xps>
<CAJFAV8zvor_rp_XORKGsQSjyifpvsW1ESfeX24cYb7_X5+F29Q@mail.gmail.com>
<f7tpnlrlh36.fsf@dhcp-25.97.bos.redhat.com>
From: Ferruh Yigit <ferruh.yigit@intel.com>
Openpgp: preference=signencrypt
Autocrypt: addr=ferruh.yigit@intel.com; prefer-encrypt=mutual; keydata=
mQINBFXZCFABEADCujshBOAaqPZpwShdkzkyGpJ15lmxiSr3jVMqOtQS/sB3FYLT0/d3+bvy
qbL9YnlbPyRvZfnP3pXiKwkRoR1RJwEo2BOf6hxdzTmLRtGtwWzI9MwrUPj6n/ldiD58VAGQ
+iR1I/z9UBUN/ZMksElA2D7Jgg7vZ78iKwNnd+vLBD6I61kVrZ45Vjo3r+pPOByUBXOUlxp9
GWEKKIrJ4eogqkVNSixN16VYK7xR+5OUkBYUO+sE6etSxCr7BahMPKxH+XPlZZjKrxciaWQb
+dElz3Ab4Opl+ZT/bK2huX+W+NJBEBVzjTkhjSTjcyRdxvS1gwWRuXqAml/sh+KQjPV1PPHF
YK5LcqLkle+OKTCa82OvUb7cr+ALxATIZXQkgmn+zFT8UzSS3aiBBohg3BtbTIWy51jNlYdy
ezUZ4UxKSsFuUTPt+JjHQBvF7WKbmNGS3fCid5Iag4tWOfZoqiCNzxApkVugltxoc6rG2TyX
CmI2rP0mQ0GOsGXA3+3c1MCdQFzdIn/5tLBZyKy4F54UFo35eOX8/g7OaE+xrgY/4bZjpxC1
1pd66AAtKb3aNXpHvIfkVV6NYloo52H+FUE5ZDPNCGD0/btFGPWmWRmkPybzColTy7fmPaGz
cBcEEqHK4T0aY4UJmE7Ylvg255Kz7s6wGZe6IR3N0cKNv++O7QARAQABtCVGZXJydWggWWln
aXQgPGZlcnJ1aC55aWdpdEBpbnRlbC5jb20+iQJUBBMBCgA+AhsDAh4BAheABQkI71rKFiEE
0jZTh0IuwoTjmYHH+TPrQ98TYR8FAlznMMQFCwkIBwMFFQoJCAsFFgIDAQAACgkQ+TPrQ98T
YR/B9Q//a57esjq996nfZVm7AsUl7zbvhN+Ojity25ib2gcSVVsAN2j6lcQS4hf6/OVvRj3q
CgebJ4o2gXR6X12UzWBJL7NE8Xpc70MvUIe0r11ykurQ9n9jUaWMjxdSqBPF93hU+Z/MZe5M
1rW5O2VJLuTJzkDw3EYUCbHOwPjeaS8Qqj3RI0LYbGthbHBIp9CsjkgsJSjTT5GQ8AQWkE7I
z+hvPx6f1rllfjxFyi4DI3jLhAI+j1Nm+l+ESyoX59HrLTHAvq4RPkLpTnGBj9gOnJ+5sVEr
GE0fcffsNcuMSkpqSEoJCPAHmChoLgezskhhsy0BiU3xlSIj1Dx2XMDerUXFOK3ftlbYNRte
HQy4EKubfZRB8H5Rvcpksom3fRBDcJT8zw+PTH14htRApU9f8I/RamQ7Ujks7KuaB7JX5QaG
gMjfPzHGYX9PfF6KIchaFmAWLytIP1t0ht8LpJkjtvUCSQZ2VxpCXwKyUzPDIF3co3tp90o7
X07uiC5ymX0K0+Owqs6zeslLY6DMxNdt8ye+h1TVkSZ5g4dCs4C/aiEF230+luL1CnejOv/K
/s1iSbXQzJNM7be3FlRUz4FdwsfKiJJF7xYALSBnSvEB04R7I2P2V9Zpudkq6DRT6HZjBeJ1
pBF2J655cdoenPBIeimjnnh4K7YZBzwOLJf2c6u76fe5Ag0EV9ZMvgEQAKc0Db17xNqtSwEv
mfp4tkddwW9XA0tWWKtY4KUdd/jijYqc3fDD54ESYpV8QWj0xK4YM0dLxnDU2IYxjEshSB1T
qAatVWz9WtBYvzalsyTqMKP3w34FciuL7orXP4AibPtrHuIXWQOBECcVZTTOdZYGAzaYzxiA
ONzF9eTiwIqe9/oaOjTwTLnOarHt16QApTYQSnxDUQljeNvKYt1lZE/gAUUxNLWsYyTT+22/
vU0GDUahsJxs1+f1yEr+OGrFiEAmqrzpF0lCS3f/3HVTU6rS9cK3glVUeaTF4+1SK5ZNO35p
iVQCwphmxa+dwTG/DvvHYCtgOZorTJ+OHfvCnSVjsM4kcXGjJPy3JZmUtyL9UxEbYlrffGPQ
I3gLXIGD5AN5XdAXFCjjaID/KR1c9RHd7Oaw0Pdcq9UtMLgM1vdX8RlDuMGPrj5sQrRVbgYH
fVU/TQCk1C9KhzOwg4Ap2T3tE1umY/DqrXQgsgH71PXFucVjOyHMYXXugLT8YQ0gcBPHy9mZ
qw5mgOI5lCl6d4uCcUT0l/OEtPG/rA1lxz8ctdFBVOQOxCvwRG2QCgcJ/UTn5vlivul+cThi
6ERPvjqjblLncQtRg8izj2qgmwQkvfj+h7Ex88bI8iWtu5+I3K3LmNz/UxHBSWEmUnkg4fJl
Rr7oItHsZ0ia6wWQ8lQnABEBAAGJAjwEGAEKACYCGwwWIQTSNlOHQi7ChOOZgcf5M+tD3xNh
HwUCXOcvZgUJBvIWKAAKCRD5M+tD3xNhHxhBD/9toXMIaPIVFd9w1nKsRDM1GE6gZe4jie8q
MJpeHB9O+936fSXA0W2X0het60wJQQ45O8TpTcxpc9nGzcE4MTaLAI3E8TjIXAO0cPqUNLyp
g0DXezmTw5BU+SKZ51+jSKOtFmzJCHOJZQaMeCHD+G3CrdUHQVQBb5AeuH3KFv9ltgDcWsc8
YO70o3+tGHwcEnyXLdrI0q05wV7ncnLdkgVo+VUN4092bNMPwYly1TZWcU3Jw5gczOUEfTY7
sgo6E/sGX3B+FzgIs5t4yi1XOweCAQ/mPnb6uFeNENEFyGKyMG1HtjwBqnftbiFO3qitEIUY
xWGQH23oKscv7i9lT0gg2D+ktzZhVWwHJVY/2vWSB9aCSWChcH2BT+lWrkwSpoPhy+almM84
Qz2wF72/d4ce4L27pSrS+vOXtXHLGOOGcAn8yr9TV0kM4aR+NbGBRXGKhG6w4lY54uNd9IBa
ARIPUhij5JSygxZCBaJKo+X64AHGkk5bXq+f0anwAMNuJXbYC/lz4DEdKmPgQGShOWNs1Y1a
N3cI87Hun/RBVwQ0a3Tr1g6OWJ6xK8cYbMcoR8NZ7L9ALMeJeuUDQR39+fEeHg/6sQN0P0mv
0sL+//BAJphCzDk8ztbrFw+JaPtgzZpRSM6JhxnY+YMAsatJRXA0WSpYP5zzl7yu/GZJIgsv VQ==
Message-ID: <c87285dd-f322-809f-7cc3-3b41593da1be@intel.com>
Date: Tue, 30 Jul 2019 15:48:31 +0100
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
Thunderbird/60.8.0
MIME-Version: 1.0
In-Reply-To: <f7tpnlrlh36.fsf@dhcp-25.97.bos.redhat.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] librte_flow_classify: fix
out-of-bounds access
X-BeenThere: stable@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches for DPDK stable branches <stable.dpdk.org>
List-Unsubscribe: <https://mails.dpdk.org/options/stable>,
<mailto:stable-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://mails.dpdk.org/archives/stable/>
List-Post: <mailto:stable@dpdk.org>
List-Help: <mailto:stable-request@dpdk.org?subject=help>
List-Subscribe: <https://mails.dpdk.org/listinfo/stable>,
<mailto:stable-request@dpdk.org?subject=subscribe>
Errors-To: stable-bounces@dpdk.org
Sender: "stable" <stable-bounces@dpdk.org>
On 7/30/2019 3:42 PM, Aaron Conole wrote:
> David Marchand <david.marchand@redhat.com> writes:
>
>> On Wed, Jul 10, 2019 at 11:49 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>>>
>>> 09/07/2019 13:09, Bernard Iremonger:
>>>> This patch fixes the out-of-bounds coverity issue by removing the
>>>> offending line of code at line 107 in rte_flow_classify_parse.c
>>>> which is never executed.
>>>>
>>>> Coverity issue: 343454
>>>>
>>>> Fixes: be41ac2a330f ("flow_classify: introduce flow classify library")
>>>> Cc: stable@dpdk.org
>>>> Signed-off-by: Bernard Iremonger <bernard.iremonger@intel.com>
>>>
>>> Applied, thanks
>>
>> We have a segfault in the unit tests since this patch.
>
> I think this patch is still correct. The issue is in the semantic of
> the flow classify pattern. It *MUST* always have a valid end marker,
> but the test passes an invalid end marker. This causes the bounds to
> exceed.
>
> So, it would be best to fix it, either by having a "failure" on unknown
> markers (f.e. -1), or by passing a length around. However, the crash
> should be expected. The fact that the previous code was also incorrect
> and resulted in no segfault is pure luck.
>
> See rte_flow_classify_parse.c:80 and test_flow_classify.c:387
>
> I would be in favor of passing the lengths of the two arrays to these
> APIs. That would let us still make use of the markers (for valid
> construction), but also let us reason about lengths in a sane way.
>
> WDYT?
>
+1, I also just replied with something very similar.
With current API the testcase is wrong, and it will crash, also the invalid
action one has exact same problem.
The API can be updated as you suggested, with a length field and testcases can
be added back.
What worries me more is the rte_flow, which uses same arguments, and open to
same errors, should we consider updating rte_flow APIs to have lengths values too?