netdev - Re: [PATCH net-next 4/5] ipv6: do not drop vrf udp multicast packets

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]

Message-ID: <09797df92f76885b0e3244a3aca68dbf07a09a3a.camel@redhat.com>
Date:   Thu, 20 Sep 2018 15:02:25 +0200
From:   Paolo Abeni <pabeni@...hat.com>
To:     Mike Manning <mmanning@...tta.att-mail.com>, netdev@...r.kernel.org
Cc:     Dewi Morgan <morgand@...tta.att-mail.com>
Subject: Re: [PATCH net-next 4/5] ipv6: do not drop vrf udp multicast packets

Hi,

On Thu, 2018-09-20 at 09:58 +0100, Mike Manning wrote:
> diff --git a/net/ipv6/ip6_input.c b/net/ipv6/ip6_input.c
> index 108f5f88ec98..fc60f297d95b 100644
> --- a/net/ipv6/ip6_input.c
> +++ b/net/ipv6/ip6_input.c
> @@ -325,9 +325,12 @@ static int ip6_input_finish(struct net *net, struct sock *sk, struct sk_buff *sk
>  {
>  	const struct inet6_protocol *ipprot;
>  	struct inet6_dev *idev;
> +	struct net_device *dev;
>  	unsigned int nhoff;
> +	int sdif = inet6_sdif(skb);
>  	int nexthdr;
>  	bool raw;
> +	bool deliver;
>  	bool have_final = false;

Please, try instead to sort the variable in reverse x-mas tree order.
>  
>  	/*
> @@ -371,9 +374,27 @@ 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) {
> +				rcu_read_lock();

AFAICS, the rcu lock is already acquired at the beginning of
ip6_input_finish(), not need to acquire it here again.
+				dev = dev_get_by_index_rcu(dev_net(skb->dev),
> +							   sdif);
> +				if (!dev) {
> +					rcu_read_unlock();
> +					kfree_skb(skb);
> +					return -ENODEV;
> +				}
> +			} else {
> +				dev = skb->dev;

The above fragment of code is a recurring pattern in this series,
perhaps adding an helper for it would reduce code duplication ?

Cheers,

Paolo