From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dpdk.org (dpdk.org [92.243.14.124]) by inbox.dpdk.org (Postfix) with ESMTP id B662FA0613 for ; Tue, 30 Jul 2019 16:42:24 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id D4B861C02F; Tue, 30 Jul 2019 16:42:22 +0200 (CEST) Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by dpdk.org (Postfix) with ESMTP id 202F51C02F; Tue, 30 Jul 2019 16:42:19 +0200 (CEST) Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id 13CA830BF1A7; Tue, 30 Jul 2019 14:42:17 +0000 (UTC) Received: from dhcp-25.97.bos.redhat.com (unknown [10.18.25.67]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 29E835D756; Tue, 30 Jul 2019 14:42:06 +0000 (UTC) From: Aaron Conole To: David Marchand Cc: Bernard Iremonger , dev , dpdk stable , Thomas Monjalon , "Singh\, Jasvinder" References: <1562670596-27129-1-git-send-email-bernard.iremonger@intel.com> <10372251.KTS5ePcUbj@xps> Date: Tue, 30 Jul 2019 10:42:05 -0400 In-Reply-To: (David Marchand's message of "Mon, 29 Jul 2019 15:09:18 +0200") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.2 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.42]); Tue, 30 Jul 2019 14:42:18 +0000 (UTC) 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 List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: stable-bounces@dpdk.org Sender: "stable" David Marchand writes: > On Wed, Jul 10, 2019 at 11:49 PM Thomas Monjalon 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 >> >> 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?