Re: [net,v2,3/4] neigh: discard invalid lladdr (b/mcast poison)

[Jakub Kicinski <kuba@kernel.org>]

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
neigh: discard invalid lladdr (b/mcast poison)

This commit adds validation to reject NDP Neighbor Solicitation and
Neighbor Advertisement messages that contain broadcast, multicast, or
null MAC addresses in the link-layer address options.

> diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
> index f6a5d8c73af9..34202a816a4f 100644
> --- a/net/ipv6/ndisc.c
> +++ b/net/ipv6/ndisc.c

[ ... ]

> @@ -1033,6 +1041,14 @@ static enum skb_drop_reason ndisc_recv_na(struct sk_buff *skb)
> 			net_dbg_ratelimited("NA: invalid link-layer address length\n");
> 			return reason;
> 		}
> +
> +		/*
> +		 * Broadcast/Multicast and zero MAC addresses should
> +		 * never be announced and accepted as llsrc address (prevent
                                                  ^^^^^

In ndisc_recv_na(), should this comment say "lltgt" (target link-layer
address) instead of "llsrc"?

Looking at the code above this block, the lladdr variable is assigned from
ndopts.nd_opts_tgt_lladdr (Target Link-Layer Address option per RFC 4861),
not nd_opts_src_lladdr (Source Link-Layer Address option). The comment
appears to have been copy-pasted from the NS handler without updating the
terminology.

> +		 * NDP B/MCAST MAC poisoning attack).
> +		 */
> +		if (dev->type == ARPHRD_ETHER && !is_valid_ether_addr(lladdr))
> +			return reason;
> 	}
-- 
pw-bot: cr