From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <pawelx.wodkowski@intel.com>
Received: from mga09.intel.com (mga09.intel.com [134.134.136.24])
 by dpdk.org (Postfix) with ESMTP id 4448E106B
 for <dev@dpdk.org>; Mon,  2 Mar 2015 15:55:47 +0100 (CET)
Received: from orsmga002.jf.intel.com ([10.7.209.21])
 by orsmga102.jf.intel.com with ESMTP; 02 Mar 2015 06:54:25 -0800
X-ExtLoop1: 1
X-IronPort-AV: E=Sophos;i="5.09,675,1418112000"; d="scan'208";a="692720156"
Received: from unknown (HELO [10.217.248.122]) ([10.217.248.122])
 by orsmga002.jf.intel.com with ESMTP; 02 Mar 2015 06:55:44 -0800
Message-ID: <54F479EF.4010706@intel.com>
Date: Mon, 02 Mar 2015 15:55:43 +0100
From: Pawel Wodkowski <pawelx.wodkowski@intel.com>
User-Agent: Mozilla/5.0 (Windows NT 6.1; WOW64;
 rv:31.0) Gecko/20100101 Thunderbird/31.5.0
MIME-Version: 1.0
To: "Wiles, Keith" <keith.wiles@intel.com>, 
 David Marchand <david.marchand@6wind.com>
References: <1425294562-26015-1-git-send-email-pawelx.wodkowski@intel.com>
 <CALwxeUuE1AjtZVpJrkJuXt6eGKEt97Rhi+-9OLC+=iuc+6FWWw@mail.gmail.com>
 <D119D21B.158BA%keith.wiles@intel.com>
In-Reply-To: <D119D21B.158BA%keith.wiles@intel.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit
Cc: "dev@dpdk.org" <dev@dpdk.org>
Subject: Re: [dpdk-dev] [PATCH] eal: prevent dereferencing NULL pointer in
 rte_eal_devargs_add()
X-BeenThere: dev@dpdk.org
X-Mailman-Version: 2.1.15
Precedence: list
List-Id: patches and discussions about DPDK <dev.dpdk.org>
List-Unsubscribe: <http://dpdk.org/ml/options/dev>,
 <mailto:dev-request@dpdk.org?subject=unsubscribe>
List-Archive: <http://dpdk.org/ml/archives/dev/>
List-Post: <mailto:dev@dpdk.org>
List-Help: <mailto:dev-request@dpdk.org?subject=help>
List-Subscribe: <http://dpdk.org/ml/listinfo/dev>,
 <mailto:dev-request@dpdk.org?subject=subscribe>
X-List-Received-Date: Mon, 02 Mar 2015 14:55:47 -0000

On 2015-03-02 15:40, Wiles, Keith wrote:
>
>
> On 3/2/15, 6:23 AM, "David Marchand" <david.marchand@6wind.com> wrote:
>
>> Hello Pawel,
>>
>> On Mon, Mar 2, 2015 at 12:09 PM, Pawel Wodkowski
>> <pawelx.wodkowski@intel.com
>>> wrote:
>>
>>> On failure devargs->args should not be accesed if devargs is NULL.
>>>
>>
>> accessed.
>>
>>
>>>
>>> Signed-off-by: Pawel Wodkowski <pawelx.wodkowski@intel.com>
>>> ---
>>>   lib/librte_eal/common/eal_common_devargs.c | 7 ++++---
>>>   1 file changed, 4 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/lib/librte_eal/common/eal_common_devargs.c
>>> b/lib/librte_eal/common/eal_common_devargs.c
>>> index 9b110f7..615945e 100644
>>> --- a/lib/librte_eal/common/eal_common_devargs.c
>>> +++ b/lib/librte_eal/common/eal_common_devargs.c
>>> @@ -124,12 +124,13 @@ rte_eal_devargs_add(enum rte_devtype devtype,
>>> const
>>> char *devargs_str)
>>>          return 0;
>>>
>>>   fail:
>>> -       if (devargs->args)
>>> -               free(devargs->args);
>>>          if (buf)
>>>                  free(buf);
>>> -       if (devargs)
>>> +       if (devargs) {
>>> +               free(devargs->args);
>
> Do you not still need to check for args being NULL before calling free?

No, there is no need for that. The same for buf. This NOP check is 
common practice in DPDK. I woul be good to clean this in whole library 
in separate patch set.

I recommend to read free() doc before doing another 'if (foo != NULL) 
free(foo)'

http://pubs.opengroup.org/onlinepubs/009695399/functions/free.html

-- 
Pawel