Re: [PATCH net] netlink: Relax attr validation for fixed length types

[David Miller <davem@davemloft.net>]

From: David Ahern <dsahern@gmail.com>
Date: Tue,  5 Dec 2017 12:55:40 -0700

> Commit 28033ae4e0f5 ("net: netlink: Update attr validation to require
> exact length for some types") requires attributes using types NLA_U* and
> NLA_S* to have an exact length. This change is exposing bugs in various
> userspace commands that are sending attributes with an invalid length
> (e.g., attribute has type NLA_U8 and userspace sends NLA_U32). While
> the commands are clearly broken and need to be fixed, users are arguing
> that the sudden change in enforcement is breaking older commands on
> newer kernels for use cases that otherwise "worked".
> 
> Relax the validation to print a warning mesage similar to what is done
> for messages containing extra bytes after parsing.
> 
> Fixes: 28033ae4e0f5 ("net: netlink: Update attr validation to require exact length for some types")
> Signed-off-by: David Ahern <dsahern@gmail.com>

Johannes, please review.

> ---
>  lib/nlattr.c | 15 +++++++++++----
>  1 file changed, 11 insertions(+), 4 deletions(-)
> 
> diff --git a/lib/nlattr.c b/lib/nlattr.c
> index 8bf78b4b78f0..6122662906c8 100644
> --- a/lib/nlattr.c
> +++ b/lib/nlattr.c
> @@ -28,8 +28,16 @@ static const u8 nla_attr_len[NLA_TYPE_MAX+1] = {
>  };
>  
>  static const u8 nla_attr_minlen[NLA_TYPE_MAX+1] = {
> +	[NLA_U8]	= sizeof(u8),
> +	[NLA_U16]	= sizeof(u16),
> +	[NLA_U32]	= sizeof(u32),
> +	[NLA_U64]	= sizeof(u64),
>  	[NLA_MSECS]	= sizeof(u64),
>  	[NLA_NESTED]	= NLA_HDRLEN,
> +	[NLA_S8]	= sizeof(s8),
> +	[NLA_S16]	= sizeof(s16),
> +	[NLA_S32]	= sizeof(s32),
> +	[NLA_S64]	= sizeof(s64),
>  };
>  
>  static int validate_nla_bitfield32(const struct nlattr *nla,
> @@ -70,10 +78,9 @@ static int validate_nla(const struct nlattr *nla, int maxtype,
>  	BUG_ON(pt->type > NLA_TYPE_MAX);
>  
>  	/* for data types NLA_U* and NLA_S* require exact length */
> -	if (nla_attr_len[pt->type]) {
> -		if (attrlen != nla_attr_len[pt->type])
> -			return -ERANGE;
> -		return 0;
> +	if (nla_attr_len[pt->type] && attrlen != nla_attr_len[pt->type]) {
> +		pr_warn_ratelimited("netlink: '%s': attribute type %d has an invalid length.\n",
> +				    current->comm, type);
>  	}
>  
>  	switch (pt->type) {
> -- 
> 2.11.0
>