netdev - Re: [patch net-next 1/2] net: sched: cls_basic: fix error path in basic

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <5481DE9A.7080100@gmail.com>
Date:	Fri, 05 Dec 2014 08:34:34 -0800
From:	John Fastabend <john.fastabend@...il.com>
To:	Eric Dumazet <eric.dumazet@...il.com>
CC:	Jiri Pirko <jiri@...nulli.us>, netdev@...r.kernel.org,
	davem@...emloft.net, jhs@...atatu.com
Subject: Re: [patch net-next 1/2] net: sched: cls_basic: fix error path in
 basic_change()

On 12/05/2014 08:03 AM, Eric Dumazet wrote:
> On Fri, 2014-12-05 at 07:29 -0800, John Fastabend wrote:
>> On 12/05/2014 06:50 AM, Jiri Pirko wrote:
>>> Signed-off-by: Jiri Pirko <jiri@...nulli.us>
>>> ---
>>>    net/sched/cls_basic.c | 5 ++---
>>>    1 file changed, 2 insertions(+), 3 deletions(-)
>>>
>>> diff --git a/net/sched/cls_basic.c b/net/sched/cls_basic.c
>>> index 7cf0a62..5aed341 100644
>>> --- a/net/sched/cls_basic.c
>>> +++ b/net/sched/cls_basic.c
>>> @@ -178,10 +178,9 @@ static int basic_change(struct net *net, struct sk_buff *in_skb,
>>>    			return -EINVAL;
>>>    	}
>>>
>>> -	err = -ENOBUFS;
>>>    	fnew = kzalloc(sizeof(*fnew), GFP_KERNEL);
>>> -	if (fnew == NULL)
>>> -		goto errout;
>>> +	if (!fnew)
>>> +		return -ENOBUFS;
>>>
>>>    	tcf_exts_init(&fnew->exts, TCA_BASIC_ACT, TCA_BASIC_POLICE);
>>>    	err = -EINVAL;
>>>
>>
>> Nice catch, thanks!
>>
>> Reviewed-by: John Fastabend <john.r.fastabend@...el.com>
>
> Sorry, but this looks a cosmetic change, right ?
>
> If it is a fix, we'd like a 'Fixes: ...' tag.
>
>
>

oops, you are right. fnew is null, free'ing the null value
is no issue at all from the error path. And if we are going
to start converting the use of

	if (foo == NULL)
to
	if (!foo)

in ./net/sched that is OK but not really worth the noise in
my opinion there are a lot of these in the code for quiet
sometime.

Sorry for the noise. I misread it as fixing a free on some
non-null value, which is not the case.

.John


-- 
John Fastabend         Intel Corporation
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html