On 9/24/18 10:13 AM, Mike Manning wrote:
> From: Dewi Morgan <morg...@vyatta.att-mail.com>
>
> For bound udp sockets in a vrf, also check the sdif to get the index
> for ingress devices enslaved to an l3mdev. Verify the multicast address
> against the enslaved rather than the l3mdev device.
>
> Signed-off-by: Dewi Morgan <morg...@vyatta.att-mail.com>
> Signed-off-by: Mike Manning <mmann...@vyatta.att-mail.com>
> ---
> net/ipv6/ip6_input.c | 24 ++++++++++++++++++++----
> net/ipv6/udp.c | 8 +++++---
> 2 files changed, 25 insertions(+), 7 deletions(-)
>
This should be 2 patches -- 1 that modifies the socket lookup to
consider and 1 that alters in the input path. They are completely
separate changes.
> diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
> index 108f5f88ec98..82ffb5cdd2ab 100644
> --- a/net/ipv6/ip6_input.c
> +++ b/net/ipv6/ip6_input.c
> @@ -324,11 +324,14 @@ void ipv6_list_rcv(struct list_head *head, struct
> packet_type *pt,
> static int ip6_input_finish(struct net *net, struct sock *sk, struct sk_buff
> *skb)
> {
> const struct inet6_protocol *ipprot;
> + int sdif = inet6_sdif(skb);
> + bool have_final = false;
> struct inet6_dev *idev;
> + struct net_device *dev;
make sdif and dev declarations local to where they are needed.
> unsigned int nhoff;
> + bool deliver;
deliver is not needed.
> int nexthdr;
> bool raw;
> - bool have_final = false;
so no need to move this one.
>
> /*
> * Parse extension headers
> @@ -371,9 +374,22 @@ static int ip6_input_finish(struct net *net, struct sock
> *sk, struct sk_buff *sk
> skb_postpull_rcsum(skb, skb_network_header(skb),
> skb_network_header_len(skb));
> hdr = ipv6_hdr(skb);
> - if (ipv6_addr_is_multicast(&hdr->daddr) &&
> - !ipv6_chk_mcast_addr(skb->dev, &hdr->daddr,
> - &hdr->saddr) &&
> +
> + /* skb->dev passed may be master dev for vrfs. */
> + if (sdif) {
> + dev = dev_get_by_index_rcu(dev_net(skb->dev),
net is a passed in argument. Why not use it?
> + sdif);
> + if (!dev) {
> + kfree_skb(skb);
> + return -ENODEV;
The rcu_read_lock() is held. I believe 'goto discard' is sufficient if
the enslaved device disappeared.
> + }
> + } else {
> + dev = skb->dev;
> + }
> +
> + deliver = ipv6_chk_mcast_addr(dev, &hdr->daddr,
> + &hdr->saddr);
> + if (ipv6_addr_is_multicast(&hdr->daddr) && !deliver &&
> !ipv6_is_mld(skb, nexthdr,
> skb_network_header_len(skb)))
> goto discard;
> }
I think the original code only needs skb->dev changed to dev making this
a much smaller patch.
