patches for DPDK stable branches
 help / color / mirror / Atom feed
From: Ferruh Yigit <ferruh.yigit@intel.com>
To: David Marchand <david.marchand@redhat.com>,
	Bernard Iremonger <bernard.iremonger@intel.com>
Cc: 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>
Subject: Re: [dpdk-stable] [dpdk-dev] [PATCH] librte_flow_classify: fix out-of-bounds access
Date: Tue, 30 Jul 2019 15:44:19 +0100	[thread overview]
Message-ID: <3c0899d0-2885-5a7c-6d6b-32fe86b15b9f@intel.com> (raw)
In-Reply-To: <CAJFAV8zvor_rp_XORKGsQSjyifpvsW1ESfeX24cYb7_X5+F29Q@mail.gmail.com>

On 7/29/2019 2:09 PM, David Marchand wrote:
> 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.
> 

Yes, Flavia able to reproduce the crash.

That testcase is testing a pattern without invalid END item, the pattern is same
as the rte_flow pattern.

Expectation is 'rte_flow_classify_validate()' function detect this wrong pattern
and return error, but this can't happen.
Function gets pointer to the patter array without any size/length information,
function walks through the list until it detects the END item, if this item is
missing there is no way to limit the walk through within the boundaries of the
array. As far as I can see this is same in the rte_flow implementation.

An invalid patter with missing END item is not valid testcase with current
implementation, I guess it wasn't crashing before by luck, unless I am missing
something here.

I suggest removing the mentioned testcase, also remove similar testcase for
action, invalid action without END action. If the API supports this later we can
add back the testcases.

Thanks,
ferruh


      parent reply	other threads:[~2019-07-30 14:44 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-07-09 11:09 [dpdk-stable] " Bernard Iremonger
2019-07-10 21:48 ` [dpdk-stable] [dpdk-dev] " Thomas Monjalon
2019-07-29 13:09   ` David Marchand
2019-07-30 14:42     ` Aaron Conole
2019-07-30 14:48       ` Ferruh Yigit
2019-07-30 15:43         ` Aaron Conole
2019-07-30 16:55           ` Ferruh Yigit
2019-07-30 17:30             ` Aaron Conole
2019-07-30 16:18         ` Adrien Mazarguil
2019-07-30 16:35           ` Ferruh Yigit
2019-07-30 17:27             ` Aaron Conole
2019-07-30 18:51               ` Adrien Mazarguil
2019-07-30 14:44     ` Ferruh Yigit [this message]

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3c0899d0-2885-5a7c-6d6b-32fe86b15b9f@intel.com \
    --to=ferruh.yigit@intel.com \
    --cc=adrien.mazarguil@6wind.com \
    --cc=bernard.iremonger@intel.com \
    --cc=david.marchand@redhat.com \
    --cc=dev@dpdk.org \
    --cc=flavia.musatescu@intel.com \
    --cc=jasvinder.singh@intel.com \
    --cc=stable@dpdk.org \
    --cc=thomas@monjalon.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).