netdev - Re: [PATCH net-next 2/3] mpls: consistently use u8 to store number of labels

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: <55CB9BBE.2000303@brocade.com>
Date:	Wed, 12 Aug 2015 20:17:18 +0100
From:	Robert Shearman <rshearma@...cade.com>
To:	Roopa Prabhu <roopa@...ulusnetworks.com>, <davem@...emloft.net>
CC:	<ebiederm@...ssion.com>, <netdev@...r.kernel.org>
Subject: Re: [PATCH net-next 2/3] mpls: consistently use u8 to store number
 of labels

On 11/08/15 22:45, Roopa Prabhu wrote:
> From: Roopa Prabhu <roopa@...ulusnetworks.com>
>
> change all types representing number of labels to u8
> to be consistent.
>
> This also changes labels to u8 in the light weight
> mpls_tunnel_encap structure. This is because the
> light weight mpls iptunnel code shares some of the label
> encoding functions like nla_get/put_labels with the af_mpls
> code.
>
> Signed-off-by: Roopa Prabhu <roopa@...ulusnetworks.com>
> ---
...
> @@ -243,11 +243,11 @@ static const struct nla_policy rtm_mpls_policy[RTA_MAX+1] = {
>   struct mpls_route_config {
>   	u32			rc_protocol;
>   	u32			rc_ifindex;
> -	u16			rc_via_table;
> -	u16			rc_via_alen;
> +	u8			rc_via_table;
> +	u8			rc_via_alen;

IMHO, it would be better to make rc_via_alen an int to avoid overflow 
which could cause false negatives in this check on the RTA_VIA attribute:

			if (cfg->rc_via_alen > MAX_VIA_ALEN)
				goto errout;

...
>   	u8			rc_via[MAX_VIA_ALEN];
> +	u8			rc_output_labels;
>   	u32			rc_label;
> -	u32			rc_output_labels;
>   	u32			rc_output_label[MAX_NEW_LABELS];
>   	u32			rc_nlflags;
>   	enum mpls_payload_type	rc_payload_type;
> @@ -751,7 +751,7 @@ int nla_put_labels(struct sk_buff *skb, int attrtype,
>   EXPORT_SYMBOL_GPL(nla_put_labels);
>
>   int nla_get_labels(const struct nlattr *nla,
> -		   u32 max_labels, u32 *labels, u32 label[])
> +		   u32 max_labels, u8 *labels, u32 label[])

How about making max_labels a u8? That would make it even more 
consistent and avoids any problem of overflow in the number of labels.

Thanks,
Rob
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html