netdev - Re: [PATCH net-next 1/2] vxlan: change vxlan_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: <20170628191801.1bc8ed67@griffin>
Date:   Wed, 28 Jun 2017 19:18:01 +0200
From:   Jiri Benc <jbenc@...hat.com>
To:     Matthias Schiffer <mschiffer@...verse-factory.net>
Cc:     davem@...emloft.net, roopa@...ulusnetworks.com, pshelar@....org,
        netdev@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH net-next 1/2] vxlan: change vxlan_validate() to use
 netlink_ext_ack for error reporting

On Tue, 27 Jun 2017 22:47:57 +0200, Matthias Schiffer wrote:
>  		if (!is_valid_ether_addr(nla_data(tb[IFLA_ADDRESS]))) {
> -			pr_debug("invalid all zero ethernet address\n");
> +			NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_ADDRESS],
> +					    "invalid ethernet address");

Could we be more specific here? This is better than nothing but still
not as helpful to the user as it could be. What about something like
"the provided ethernet address is not unicast"?

> -		if (mtu < ETH_MIN_MTU || mtu > ETH_MAX_MTU)
> +		if (mtu < ETH_MIN_MTU || mtu > ETH_MAX_MTU) {
> +			NL_SET_ERR_MSG_ATTR(extack, tb[IFLA_MTU],
> +					    "invalid MTU");

"MTU must be between 68 and 65535"

> -		if (id >= VXLAN_N_VID)
> +		if (id >= VXLAN_N_VID) {
> +			NL_SET_ERR_MSG_ATTR(extack, data[IFLA_VXLAN_ID],
> +					    "invalid VXLAN ID");

"VXLAN ID must be lower than 16777216"

>  		if (ntohs(p->high) < ntohs(p->low)) {
> -			pr_debug("port range %u .. %u not valid\n",
> -				 ntohs(p->low), ntohs(p->high));
> +			NL_SET_ERR_MSG_ATTR(extack, data[IFLA_VXLAN_PORT_RANGE],
> +					    "port range not valid");

Since you're getting rid of the values output, I'd rather suggest more
explicit "the first value of the port range must not be higher than the
second value" or so. Shorter wording is welcome :-)

Thanks,

 Jiri