netdev - Re: [PATCH net-next 1/6] net: sched: sch_api: handle generic qdisc errors

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]

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