From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from dispatch1-us1.ppe-hosted.com (dispatch1-us1.ppe-hosted.com [67.231.154.164]) by dpdk.org (Postfix) with ESMTP id 1DD88201 for ; Wed, 7 Nov 2018 07:30:24 +0100 (CET) X-Virus-Scanned: Proofpoint Essentials engine Received: from webmail.solarflare.com (uk.solarflare.com [193.34.186.16]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-SHA384 (256/256 bits)) (No client certificate requested) by mx1-us4.ppe-hosted.com (Proofpoint Essentials ESMTP Server) with ESMTPS id DBED3B40072; Wed, 7 Nov 2018 06:30:22 +0000 (UTC) Received: from [192.168.38.17] (91.220.146.112) by ukex01.SolarFlarecom.com (10.17.10.4) with Microsoft SMTP Server (TLS) id 15.0.1395.4; Wed, 7 Nov 2018 06:30:18 +0000 To: Stephen Hemminger , References: <20181106193005.5383-1-stephen@networkplumber.org> <20181106193005.5383-2-stephen@networkplumber.org> From: Andrew Rybchenko Message-ID: <5d2c4750-b418-c4fe-4e20-d7cc7ca4efe7@solarflare.com> Date: Wed, 7 Nov 2018 09:30:13 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.2.1 MIME-Version: 1.0 In-Reply-To: <20181106193005.5383-2-stephen@networkplumber.org> Content-Language: en-GB X-Originating-IP: [91.220.146.112] X-ClientProxiedBy: ocex03.SolarFlarecom.com (10.20.40.36) To ukex01.SolarFlarecom.com (10.17.10.4) X-TM-AS-Product-Ver: SMEX-12.5.0.1300-8.5.1010-24208.000 X-TM-AS-Result: No-11.948500-8.000000-10 X-TMASE-MatchedRID: oHOSwQSJZWgOwH4pD14DsPHkpkyUphL9QKuv8uQBDjp0TRq4bcxmHymu O0CFu4McT/ci3LQKeFGk0gX8yHxpHIvTpuEjuIpkyeVujmXuYYVY8mfhSYy5UvkBcyaY6pkPiO9 fzG806wuTkFhpAoMNtadCIx24gQ821j8vL5Nt0zIcsSroYI5AVv+UEb65dgmQXvMXWLuxwsracD 3TabIC9deVzSU9Cb8iNQBtSQ2jGTobGMp+f0k4HZ3iEJrvFJmhsEf8CpnIYtnfc2Xd6VJ+ysHtH Emxq9+YAI9v+AVVZW2bkXkfwEV3dsXahuIx/Hfl9UVHiwLx0/JCX8V1FiRRksI9dI7bfY/v807t +VEWpeYgZIcq73TEpZJTzjwz+Gzox7hajJv6RUXIOn6NK8S1ayXdp9l6EkRZ52VTYrkmb1ssyVh q83/5UjU3HPhRrRy9YLaaleRz6NvfW6mMWTtZdOQYBHVKqgDUfS0Ip2eEHny+qryzYw2E8LLn+0 Vm71LcY2BsXShENnS7bObxVEjIindnKV6XWPMidHAuXWBL35Nh6bh8XfW+a3OSAg1gDcV8O+UxE VnhxUdFMNjInlwshq4m7EtTQ3apgHGD4BZq+T5AKXfSCl3mbw== X-TM-AS-User-Approved-Sender: Yes X-TM-AS-User-Blocked-Sender: No X-TMASE-Result: 10--11.948500-8.000000 X-TMASE-Version: SMEX-12.5.0.1300-8.5.1010-24208.000 X-MDID: 1541572223-SsSthNEjvwC6 Content-Type: text/plain; charset="utf-8"; format=flowed Content-Transfer-Encoding: 7bit X-Content-Filtered-By: Mailman/MimeDel 2.1.15 Subject: Re: [dpdk-dev] [PATCH 1/4] net/failsafe: avoid rte_memcpy if rte_realloc fails 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: , X-List-Received-Date: Wed, 07 Nov 2018 06:30:24 -0000 On 11/6/18 10:30 PM, Stephen Hemminger wrote: > There is a potential issue seen by static tools if number of multicast > addresses is zero, and rte_realloc of zero size fails (ie returns NULL). > This won't happen in real world for a couple of reasons: Azure doesn't > support multicast (ie this is dead code); Is it guaranteed that Azure is the only user of the code? Sorry, it does not sound like an argument at all. > and rte_realloc of zero size > will never fail, but safe to just always return -ENOMEM of realloc fails. It is false statement. If ptr is NULL, rte_malloc() is used which explicitly returns NULL if size is 0. > Coverity issue: 323487 It is 100% false alarm from Coverity. rte_memcpy() does nothing if size is 0 and it is zero if number of addresses is zero. If we really want to cope with it (I'm not sure), just do rte_memcpy() in else branch. And it should explained in the comment that it is required to address false issue from static analysis tool only. Other option is to add check for dummy set (zero number of addresses when it is already zero, but it is extra lines of code and extra logic which is not actually required here. So, more harm from my point of view. > Fixes: 901efc0da925 ("net/failsafe: support multicast address list set") > Signed-off-by: Stephen Hemminger > --- > drivers/net/failsafe/failsafe_ops.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/failsafe/failsafe_ops.c b/drivers/net/failsafe/failsafe_ops.c > index 7f8bcd4c69f4..a20953a662e1 100644 > --- a/drivers/net/failsafe/failsafe_ops.c > +++ b/drivers/net/failsafe/failsafe_ops.c > @@ -1155,7 +1155,7 @@ fs_set_mc_addr_list(struct rte_eth_dev *dev, > > mcast_addrs = rte_realloc(PRIV(dev)->mcast_addrs, > nb_mc_addr * sizeof(PRIV(dev)->mcast_addrs[0]), 0); > - if (mcast_addrs == NULL && nb_mc_addr > 0) { > + if (mcast_addrs == NULL) { > ret = -ENOMEM; > goto rollback; > }