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 3AF98A00BE; Mon, 27 Apr 2020 15:52:32 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id A16251D443; Mon, 27 Apr 2020 15:52:31 +0200 (CEST) Received: from us-smtp-1.mimecast.com (us-smtp-1.mimecast.com [207.211.31.81]) by dpdk.org (Postfix) with ESMTP id 1F7BA1D443 for ; Mon, 27 Apr 2020 15:52:30 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587995549; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=0QKmKgbFIw1KYq0Sn7iCsBQ4NL0Q4snmJSxPxeqLiis=; b=ORoyyv6erigj5GmXTE1WBHyMIKx2kxPfIM2yCMOAbSlsEtZvaKwMONKkbjE2cKxjzXPXGi eJVBwDo6UeZ1hK40dNFvyF5GRp9Cyyar8GOlaotGKHsF4zfRz+SszlP/qJzDPjez5IHbt+ Cyw4ohfxuagVeK8LVp++4BkTn0RM/xs= Received: from mail-ua1-f70.google.com (mail-ua1-f70.google.com [209.85.222.70]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-3-gdn7nDw3MMOQ3Agh0WIc5A-1; Mon, 27 Apr 2020 09:52:21 -0400 X-MC-Unique: gdn7nDw3MMOQ3Agh0WIc5A-1 Received: by mail-ua1-f70.google.com with SMTP id z20so9709779uag.19 for ; Mon, 27 Apr 2020 06:52:21 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=ohml0mdNRKPXIP1ITw1uwAlJKNwAWgXoRXMjnN58E8g=; b=t3wkCHDe9u9X9PCo1QSxhqhMZyu1cmbBnVbFRUYNEfGF8uHUlmcuhgC663ZbntD75y 0HGuQOD9j42eFfwc1gFlfhfY90HUw65JBKXT6ilBDhMigkLD0i0fjOScRPVmje1B+7nL wGqpen/YX3jNgFgHTVZBpZCBptUSva7uk7f5hgB+r96lwafvxNgLQ+RjDWx3RDjkDzrG IJmIZAwCnLKCkV7lsUnpKvwfr4ex6w97DlGwrHsVGS9t/tBjmsQSQfHd0roo1DWhoy/j 6UGqPj1P7TghY19OAtm7MVP4NlFcZLbptBt7YyuYYnCPrmkpZ6qHx5pFCqcpRuVYDFEG tohg== X-Gm-Message-State: AGi0PuY+s13llbJKFsHCOuuc22GjrZsRI+0SwjWl/LVbcSHEfV/04sF5 72XxyfnVY/3sz9NshCa8Siy/ByJke68mM9nL8Mn9WerKmcJofJcPA3FZLkdI/Sjrpy7i1Qf1HIP Nl3t3/sHnbqEetv2Xp80= X-Received: by 2002:a67:d817:: with SMTP id e23mr12545067vsj.39.1587995540670; Mon, 27 Apr 2020 06:52:20 -0700 (PDT) X-Google-Smtp-Source: APiQypK78PPjtsBkn62rvzJguJKN27OBTRxGnJVv5pBH/ReT6hzbF8W/G4+7UIy4wX4P+zMPu9lvqE+Ds6GHUnqq+oQ= X-Received: by 2002:a67:d817:: with SMTP id e23mr12545048vsj.39.1587995540413; Mon, 27 Apr 2020 06:52:20 -0700 (PDT) MIME-Version: 1.0 References: <20200427120424.22728-1-skori@marvell.com> In-Reply-To: From: David Marchand Date: Mon, 27 Apr 2020 15:52:09 +0200 Message-ID: To: Sunil Kumar Kori Cc: Jerin Jacob Kollanukkaran , dev X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Subject: Re: [dpdk-dev] [EXT] Re: [PATCH] eal/trace: fix coverity issues X-BeenThere: dev@dpdk.org X-Mailman-Version: 2.1.15 Precedence: list List-Id: DPDK patches and discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: dev-bounces@dpdk.org Sender: "dev" On Mon, Apr 27, 2020 at 3:46 PM Sunil Kumar Kori wrote: > > >-----Original Message----- > >From: David Marchand > >Sent: Monday, April 27, 2020 5:56 PM > >To: Sunil Kumar Kori > >Cc: Jerin Jacob Kollanukkaran ; dev > >Subject: [EXT] Re: [dpdk-dev] [PATCH] eal/trace: fix coverity issues > > > >External Email > > > >---------------------------------------------------------------------- > >On Mon, Apr 27, 2020 at 2:04 PM Sunil Kumar Kori > >wrote: > >> > >> Pointer was being dereferenced without NULL checking. > >> > >> Coverity issue: 357768 > >> > >> Fixes: 8c8066ea6a7b ("trace: add trace mode configuration parameter") > >> > >> Signed-off-by: Sunil Kumar Kori > >> --- > >> lib/librte_eal/common/eal_common_trace_utils.c | 3 ++- > >> 1 file changed, 2 insertions(+), 1 deletion(-) > >> > >> diff --git a/lib/librte_eal/common/eal_common_trace_utils.c > >> b/lib/librte_eal/common/eal_common_trace_utils.c > >> index fce8892c3..119e97119 100644 > >> --- a/lib/librte_eal/common/eal_common_trace_utils.c > >> +++ b/lib/librte_eal/common/eal_common_trace_utils.c > >> @@ -227,15 +227,16 @@ int > >> eal_trace_mode_args_save(const char *optarg) { > >> struct trace *trace =3D trace_obj_get(); > >> - size_t len =3D strlen(optarg); > >> unsigned long tmp; > >> char *pattern; > >> + size_t len; > >> > >> if (optarg =3D=3D NULL) { > >> trace_err("no optarg is passed"); > >> return -EINVAL; > >> } > >> > >> + len =3D strlen(optarg); > >> if (len =3D=3D 0) { > >> trace_err("value is not provided with option"); > >> return -EINVAL; > > > >I was looking at some gcc 10 complaints on string manipulation later > >in eal_trace_dir_args_save(). > > > >https://urldefense.proofpoint.com/v2/url?u=3Dhttps- > >3A__build.opensuse.org_package_live-5Fbuild-5Flog_home-3Admarchan- > >3Abranches-3Ahome-3Abluca-3Adpdk_dpdk_Fedora-5FRawhide_x86- > >5F64&d=3DDwIFaQ&c=3DnKjWec2b6R0mOyPaz7xtfQ&r=3DdXeXaAMkP5COgn1zxHMy > >aF1_d9IIuq6vHQO6NrIPjaE&m=3DNZ72Sr2OMEYZD7PIY59lshlAxZJJJepF5oxbHv0j > >5Zg&s=3DyOCA3PfhZojqJv0iVKlzeqM7tYGVv0jjrnVcajUx_qA&e=3D > > > >[ 126s] CC rte_malloc.o > >[ 127s] /home/abuild/rpmbuild/BUILD/dpdk- > >1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c: > >In function 'eal_trace_dir_args_save': > >[ 127s] /home/abuild/rpmbuild/BUILD/dpdk- > >1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c:29 > >0:24: > >error: 'sprintf' may write a terminating nul past the end of the > >destination [-Werror=3Dformat-overflow=3D] > >[ 127s] 290 | sprintf(dir_path, "%s/", optarg); > >[ 127s] | ^ > >[ 127s] /home/abuild/rpmbuild/BUILD/dpdk- > >1587835122.b13ace300/lib/librte_eal/common/eal_common_trace_utils.c:29 > >0:2: > >note: 'sprintf' output between 2 and 4097 bytes into a destination of > >size 4096 > >[ 127s] 290 | sprintf(dir_path, "%s/", optarg); > >[ 127s] | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > >[ 127s] cc1: all warnings being treated as errors > > > > > >Could we use asprintf in all this code and avoid malloc + sprintf ? > > > As I understood from above warnings/errors, real problem is writing beyon= d destination i.e. dir_path. > If this is the case then it can be simply handled using snprintf(); with = correct "size" information. > Suggested code changes are correct. I am just trying to achieve this with= lesser code changes. > > Also I think, fix for this should be a separate patch. > Suggestions please ? If the code was not combining this malloc + sprintf + wrong checks (caught by coverity), we would avoid both issues and it would be more consistent. No more suggestion from me. --=20 David Marchand