netdev - Re: [PATCH net-next 2/8] net: sched: cls

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:   Tue, 16 Jan 2018 19:19:50 -0500
From:   Jamal Hadi Salim <jhs@...atatu.com>
To:     David Ahern <dsahern@...il.com>,
        Alexander Aring <aring@...atatu.com>
Cc:     xiyou.wangcong@...il.com, jiri@...nulli.us, davem@...emloft.net,
        netdev@...r.kernel.org, kernel@...atatu.com
Subject: Re: [PATCH net-next 2/8] net: sched: cls_api: handle generic cls
 errors

On 18-01-16 06:58 PM, David Ahern wrote:
> On 1/16/18 9:20 AM, Alexander Aring wrote:

>>   		}
>>   
>>   		if (n->nlmsg_type != RTM_NEWTFILTER ||
>>   		    !(n->nlmsg_flags & NLM_F_CREATE)) {
>> +			NL_SET_ERR_MSG(extack, "Need both RTM_NEWTFILTER and NLM_F_CREATE to create a new filter");
> 
> that does not seem the right message. tc_ctl_tfilter is overloaded for
> new, delete and get so the response here needs to reflect that. I
> believe in this case the user did not specify a valid chain.
> 

Are you sure you are looking at the correct code?
It is a create message that is at stake here.
A create has to have RTM_NEWTFILTER and NLM_F_CREATE

> Also, the messages are targeted at users not developers, so no code
> jargon / API references.

Generally true, but should this rule really be scripture?
The main user here is tc in  user space and it doesnt make mistakes
in this case i.e we will  never see this error with tc because a
create will always have those two set correctly; OTOH, a developer
writing some new app is more likely to make this mistake (in which
case this message is very helpful).

cheers,
jamal