netdev - Re: [PATCH net-next] docs: netlink: clarify the historical baggage of Netlink flags

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]

Date:   Wed, 28 Sep 2022 17:19:10 +0200
From:   Nicolas Dichtel <nicolas.dichtel@...nd.com>
To:     Jakub Kicinski <kuba@...nel.org>,
        Nikolay Aleksandrov <razor@...ckwall.org>
Cc:     Florent Fourcot <florent.fourcot@...irst.fr>,
        netdev@...r.kernel.org, davem@...emloft.net, edumazet@...gle.com,
        pabeni@...hat.com, Johannes Berg <johannes@...solutions.net>,
        Pablo Neira Ayuso <pablo@...filter.org>,
        Florian Westphal <fw@...len.de>,
        Jamal Hadi Salim <jhs@...atatu.com>,
        Jacob Keller <jacob.e.keller@...el.com>,
        Guillaume Nault <gnault@...hat.com>,
        Hangbin Liu <liuhangbin@...il.com>
Subject: Re: [PATCH net-next] docs: netlink: clarify the historical baggage of
 Netlink flags

Le 28/09/2022 à 16:37, Jakub Kicinski a écrit :
[snip]
> I was very tempted to send a patch which would validate the top
> byte of flags in genetlink for new commands, this way we may some day
> find a truly common (as in enforced by the code) use for the bits. 
> 
> WDYT?
I think it's worth trying it. In the worst case, this could be reverted.
It will help to see how people are using these flags.
The same kind of patch for rtnetlink could be interesting before someone adds a
new flag that overlaps.

> 
> diff --git a/net/netlink/genetlink.c b/net/netlink/genetlink.c
> index 7c136de117eb..0fbaed49e23f 100644
> --- a/net/netlink/genetlink.c
> +++ b/net/netlink/genetlink.c
> @@ -739,6 +739,22 @@ static int genl_family_rcv_msg_doit(const struct genl_family *family,
>  	return err;
>  }
>  
> +static int genl_header_check(struct nlmsghdr *nlh, struct genlmsghdr *hdr)
> +{
> +	if (hdr->reserved)
> +		return -EINVAL;
> +
> +	/* Flags - lower byte check */
> +	if (nlh->nlmsg_flags & 0xff & ~(NLM_F_REQUEST | NLM_F_ACK | NLM_F_ECHO))
> +		return -EINVAL;
> +	/* Flags - higher byte check */
> +	if (nlh->nlmsg_flags & 0xff00 &&
> +	    nlh->nlmsg_flags & 0xff00 != NLM_F_DUMP)
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
>  static int genl_family_rcv_msg(const struct genl_family *family,
>  			       struct sk_buff *skb,
>  			       struct nlmsghdr *nlh,
> @@ -757,7 +773,7 @@ static int genl_family_rcv_msg(const struct genl_family *family,
>  	if (nlh->nlmsg_len < nlmsg_msg_size(hdrlen))
>  		return -EINVAL;
>  
> -	if (hdr->cmd >= family->resv_start_op && hdr->reserved)
> +	if (hdr->cmd >= family->resv_start_op && genl_header_check(nlh, hdr))
>  		return -EINVAL;
>  
>  	if (genl_get_cmd(hdr->cmd, family, &op))