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 6D9CCA00BE; Mon, 27 Apr 2020 14:26:24 +0200 (CEST) Received: from [92.243.14.124] (localhost [127.0.0.1]) by dpdk.org (Postfix) with ESMTP id 0CBE21D16D; Mon, 27 Apr 2020 14:26:24 +0200 (CEST) Received: from us-smtp-1.mimecast.com (us-smtp-delivery-1.mimecast.com [205.139.110.120]) by dpdk.org (Postfix) with ESMTP id 10D561D16C for ; Mon, 27 Apr 2020 14:26:22 +0200 (CEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1587990382; 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=J8KyuxMU8NK3EwTNo9UI+sL94oVVhpz01F6kmlHoaGU=; b=DHnkywCck0jsLdxSlo08PtXCmserpEiwQfButWh7PeZkNbzOFYlD/xnf0bzs0m+MbbRyvn LWW5fjrcWrD9PdwKvcR5/xWJeiQ0DYfu7W7TJ3xT2QAvv+V1YH5uxoYy+EYiD4zE7dAEV2 ZoSwzXQ98oM4HZc3Kd6AEZZPZCOF27s= Received: from mail-ua1-f71.google.com (mail-ua1-f71.google.com [209.85.222.71]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-310-j3XPYyoNOBe_IH_4-bzpGw-1; Mon, 27 Apr 2020 08:26:18 -0400 X-MC-Unique: j3XPYyoNOBe_IH_4-bzpGw-1 Received: by mail-ua1-f71.google.com with SMTP id v27so9526139uaa.22 for ; Mon, 27 Apr 2020 05:26:18 -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=6WMicP8XuVWDcRiXMcLJigBo/gDsmNhiL50sU0rGNWE=; b=cxuAiFQP5ZkOoVnmgrusNE6czt7qJWY4MmMvF84O/jtEeEzLOs2aSAhKzUTC2ZxFVC 6DCSsdhXPixPG9UjqU9sKzOpWdY3hDXJIbUfi9qwNmQSajbPTiAxrQn61qR+O3dhxR0R FTe7hD0VYDoPPNMtOWSvt16nW2ZPrhJPxdTLGzJWoxm2S0N8rM7a5vM051quvIikIEy3 /+muw4Gc87cOi/xe3UfZh9pzau3W/R6++yqiT9UBzRAoVgdDEX2J3PmKmy1PHPwpSanB cmRE2OQuxRhPfi40KpPzsTMLwNvfHBBeAb/1QOwi0iZV3Je34jUwIS6pbtLUOsOq4SqV s+Sg== X-Gm-Message-State: AGi0PuaNEeTt6txbI+ZqumrKDxSanNZfxuozn1kEbCa0P4PAVPXgkuid wSQB89b7tZVF3cERwCxvx67jgFCzZ+2R1uFYSrFu7K0Q5Y8MEY9eP68f5DqUNXLqwQTAMLZS0dd MHF/Ll60l0+DfJC1L7WU= X-Received: by 2002:ab0:485:: with SMTP id 5mr15697347uaw.86.1587990377773; Mon, 27 Apr 2020 05:26:17 -0700 (PDT) X-Google-Smtp-Source: APiQypL8ff1hRI7QW7YqwvcJPa8FmEJUzC7Gtb3rLnJhkCtKIx48fB2zTfUXgNfBvnTzyqemWkP8/yybU2cKZXUkhC4= X-Received: by 2002:ab0:485:: with SMTP id 5mr15697317uaw.86.1587990377463; Mon, 27 Apr 2020 05:26:17 -0700 (PDT) MIME-Version: 1.0 References: <20200427120424.22728-1-skori@marvell.com> In-Reply-To: <20200427120424.22728-1-skori@marvell.com> From: David Marchand Date: Mon, 27 Apr 2020 14:26:06 +0200 Message-ID: To: Sunil Kumar Kori Cc: Jerin Jacob , 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] [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 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://build.opensuse.org/package/live_build_log/home:dmarchan:branches:ho= me:bluca:dpdk/dpdk/Fedora_Rawhide/x86_64 [ 126s] CC rte_malloc.o [ 127s] /home/abuild/rpmbuild/BUILD/dpdk-1587835122.b13ace300/lib/librte_e= al/common/eal_common_trace_utils.c: In function 'eal_trace_dir_args_save': [ 127s] /home/abuild/rpmbuild/BUILD/dpdk-1587835122.b13ace300/lib/librte_e= al/common/eal_common_trace_utils.c:290: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_e= al/common/eal_common_trace_utils.c:290: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 ? Something like the _untested_ snippet? diff --git a/lib/librte_eal/common/eal_common_trace_utils.c b/lib/librte_eal/common/eal_common_trace_utils.c index fce8892c38..4769bade97 100644 --- a/lib/librte_eal/common/eal_common_trace_utils.c +++ b/lib/librte_eal/common/eal_common_trace_utils.c @@ -267,8 +267,7 @@ int eal_trace_dir_args_save(char const *optarg) { struct trace *trace =3D trace_obj_get(); - uint32_t size =3D sizeof(trace->dir); - char *dir_path =3D NULL; + char *dir_path; int rc; if (optarg =3D=3D NULL) { @@ -276,19 +275,18 @@ eal_trace_dir_args_save(char const *optarg) return -EINVAL; } - if (strlen(optarg) >=3D size) { - trace_err("input string is too big"); - return -ENAMETOOLONG; - } - - dir_path =3D (char *)calloc(1, size); - if (dir_path =3D=3D NULL) { - trace_err("fail to allocate memory"); + rc =3D asprintf(&dir_path, "%s/", optarg); + if (rc =3D=3D -1) { + trace_err("failed to copy directory: %s", strerror(errno)); return -ENOMEM; } - sprintf(dir_path, "%s/", optarg); - rc =3D trace_dir_update(dir_path); + if ((size_t)rc >=3D sizeof(trace->dir)) { + trace_err("input string is too big"); + rc =3D -ENAMETOOLONG; + } else { + rc =3D trace_dir_update(dir_path); + } free(dir_path); return rc; --=20 David Marchand