netdev - Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to use netlink_ext

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: <0af5e8f9-36cf-8aed-b431-ffc19eac4c8f@gmail.com>
Date:   Fri, 11 Aug 2017 11:17:54 -0600
From:   David Ahern <dsahern@...il.com>
To:     Jiri Benc <jbenc@...hat.com>
Cc:     Roopa Prabhu <roopa@...ulusnetworks.com>,
        Girish Moodalbail <girish.moodalbail@...cle.com>,
        pravin shelar <pshelar@....org>,
        "davem@...emloft.net" <davem@...emloft.net>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        Matthias Schiffer <mschiffer@...verse-factory.net>
Subject: Re: [PATCH net-next v2] vxlan: change vxlan_[config_]validate() to
 use netlink_ext_ack for error reporting

On 8/11/17 11:11 AM, Jiri Benc wrote:
> On Fri, 11 Aug 2017 10:56:57 -0600, David Ahern wrote:
>> I would argue none of those messages are sufficient. The message should
>> tell the user what is missing.
> 
> Good point.
> 
> I guess "The IFLA_INFO_DATA attribute is missing" would be a better
> message. It can happen only when you're implementing your own
> management tool, it's not that you'll get this message out of the ip
> tool, thus referring to netlink attributes should be okay.
> 
>> What is the point of the !data check anyway? Based on the rest of the
>> validate function neither IFLA_VXLAN_ID or IFLA_VXLAN_PORT_RANGE are
>> required attributes.
> 
> The newlink callback assumes data is not NULL, i.e. IFLA_INFO_DATA is
> present. It would crash otherwise.


What if a user adds IFLA_INFO_DATA but adds no vxlan attributes under
it? Still not a valid config, but it passes the !data check.

Whatever attributes are required but missing should be the message
returned.