netdev - Re: [PATCH net-next 2/3] net, neigh: Use NLA_POLICY_MASK helper for NDA_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, 13 Oct 2021 21:13:40 -0600
From:   David Ahern <dsahern@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>, davem@...emloft.net,
        kuba@...nel.org
Cc:     roopa@...dia.com, dsahern@...nel.org, m@...bda.lt,
        john.fastabend@...il.com, netdev@...r.kernel.org,
        bpf@...r.kernel.org
Subject: Re: [PATCH net-next 2/3] net, neigh: Use NLA_POLICY_MASK helper for
 NDA_FLAGS_EXT attribute

On 10/13/21 7:21 AM, Daniel Borkmann wrote:
> Instead of open-coding a check for invalid bits in NTF_EXT_MASK, we can just
> use the NLA_POLICY_MASK() helper instead, and simplify NDA_FLAGS_EXT sanity
> check this way.
> 
> Suggested-by: Jakub Kicinski <kuba@...nel.org>
> Signed-off-by: Daniel Borkmann <daniel@...earbox.net>
> ---
>  net/core/neighbour.c | 6 +-----
>  1 file changed, 1 insertion(+), 5 deletions(-)
> 
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 4fc601f9cd06..922b9ed0fe76 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -1834,7 +1834,7 @@ const struct nla_policy nda_policy[NDA_MAX+1] = {
>  	[NDA_MASTER]		= { .type = NLA_U32 },
>  	[NDA_PROTOCOL]		= { .type = NLA_U8 },
>  	[NDA_NH_ID]		= { .type = NLA_U32 },
> -	[NDA_FLAGS_EXT]		= { .type = NLA_U32 },
> +	[NDA_FLAGS_EXT]		= NLA_POLICY_MASK(NLA_U32, NTF_EXT_MASK),
>  	[NDA_FDB_EXT_ATTRS]	= { .type = NLA_NESTED },
>  };
>  
> @@ -1936,10 +1936,6 @@ static int neigh_add(struct sk_buff *skb, struct nlmsghdr *nlh,
>  	if (tb[NDA_FLAGS_EXT]) {
>  		u32 ext = nla_get_u32(tb[NDA_FLAGS_EXT]);
>  
> -		if (ext & ~NTF_EXT_MASK) {
> -			NL_SET_ERR_MSG(extack, "Invalid extended flags");
> -			goto out;
> -		}
>  		BUILD_BUG_ON(sizeof(neigh->flags) * BITS_PER_BYTE <
>  			     (sizeof(ndm->ndm_flags) * BITS_PER_BYTE +
>  			      hweight32(NTF_EXT_MASK)));
> 

I get that NLA_POLICY_MASK wants to standardize the logic, but the
generic extack message "reserved bit set" is less useful than the one here.