lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  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]
Date:   Thu, 7 Dec 2017 07:04:25 -0500
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     David Ahern <dsahern@...il.com>,
        Alexander Aring <aring@...atatu.com>, davem@...emloft.net
Cc:     xiyou.wangcong@...il.com, jiri@...nulli.us, netdev@...r.kernel.org,
        kernel@...atatu.com
Subject: Re: [PATCH net-next 1/6] net: sched: sch_api: handle generic qdisc
 errors

On 17-12-07 12:28 AM, David Ahern wrote:

>>   
>> -	err = nla_parse_nested(tb, TCA_STAB_MAX, opt, stab_policy, NULL);
>> -	if (err < 0)
>> +	err = nla_parse_nested(tb, TCA_STAB_MAX, opt, stab_policy, extack);
>> +	if (err < 0) {
>> +		NL_SET_ERR_MSG(extack, "Failed to parse stab netlink msg");
> 
> you don't want to set extack here; it should be set in nla_parse_nested
> since it is passed as an arg.
> 

Can you really have a generic message in nla_parse(_nested)? What
would it say?
Note: the bad attribute is saved in the bowels of nla_parsing.

-
>>   	stab = kmalloc(sizeof(*stab) + tsize * sizeof(u16), GFP_KERNEL);
>> -	if (!stab)
>> +	if (!stab) {
>> +		NL_SET_ERR_MSG(extack, "Failed to allocate memory for stab data");
> 
> ENOMEM does not need a text message. Which memory allocation failed is
> not really relevant.
> 

YMMV.
On the one hand it is useful to distinguish which allocation
in the code path failed(if there was a bug for example).
On the other hand you could argue an alloc failure is as dramatic
as the "sky is falling" - doesnt matter what part of the globe it is 
falling at. If the cost of sending or not is the same why not include
the message?

cheers,
jamal

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux - Powered by OpenVZ