Re: [tipc-discussion] [net-next] tipc: do sanity check payload of a netlink message

[Jon M. <jm...@re...>]

On 12/11/20 6:32 AM, Hoang Huu Le wrote:
> From: Hoang Le <hoa...@de...>
>
> We initialize nlmsghdr without any payload in tipc_nl_compat_dumpit(),
> then, result of calling parse attributes always fails and return with
> '-EINVAL' error.
When we initialize nlmsghdr with no payload inside 
tipc_nl_compat_dumpit() the
parsing function returns -EINVAL. We fix it by making the parsing call 
conditional.
>
> To fix error returning when parsing attributes of a netlink message,
> we do a sanity check the length of message payload.
>
> Signed-off-by: Hoang Le <hoa...@de...>
> ---
>   net/tipc/netlink_compat.c | 12 +++++++-----
>   1 file changed, 7 insertions(+), 5 deletions(-)
>
> diff --git a/net/tipc/netlink_compat.c b/net/tipc/netlink_compat.c
> index 82f154989418..5a1ce64039f7 100644
> --- a/net/tipc/netlink_compat.c
> +++ b/net/tipc/netlink_compat.c
> @@ -213,12 +213,14 @@ static int __tipc_nl_compat_dumpit(struct tipc_nl_compat_cmd_dump *cmd,
>   	}
>   
>   	info.attrs = attrbuf;
> -	err = nlmsg_parse_deprecated(cb.nlh, GENL_HDRLEN, attrbuf,
> -				     tipc_genl_family.maxattr,
> -				     tipc_genl_family.policy, NULL);
> -	if (err)
> -		goto err_out;
>   
> +	if (nlmsg_len(cb.nlh) > 0) {
> +		err = nlmsg_parse_deprecated(cb.nlh, GENL_HDRLEN, attrbuf,
> +					     tipc_genl_family.maxattr,
> +					     tipc_genl_family.policy, NULL);
> +		if (err)
> +			goto err_out;
> +	}
>   	do {
>   		int rem;
>   
Acked-by: Jon Maloy <jm...@re...>