ACK: [PATCH 1/1] ipv4: Don't do expensive useless work during inetdev destroy.

[Chris J Arges]

Backport looks correct, fixes a local DOS.
--chris

On Mon, Apr 18, 2016 at 03:05:00PM +0100, Luis Henriques wrote:
> From: "David S. Miller" <davem at davemloft.net>
> 
> When an inetdev is destroyed, every address assigned to the interface
> is removed.  And in this scenerio we do two pointless things which can
> be very expensive if the number of assigned interfaces is large:
> 
> 1) Address promotion.  We are deleting all addresses, so there is no
>    point in doing this.
> 
> 2) A full nf conntrack table purge for every address.  We only need to
>    do this once, as is already caught by the existing
>    masq_dev_notifier so masq_inet_event() can skip this.
> 
> Reported-by: Solar Designer <solar at openwall.com>
> Signed-off-by: David S. Miller <davem at davemloft.net>
> Tested-by: Cyrill Gorcunov <gorcunov at openvz.org>
> (backported from commit fbd40ea0180a2d328c5adc61414dc8bab9335ce2)
> [ luis: file rename: nf_nat_masquerade_ipv4.c -> ipt_MASQUERADE.c ]
> CVE-2016-3156
> BugLink: https://bugs.launchpad.net/bugs/1558847
> Signed-off-by: Luis Henriques <luis.henriques at canonical.com>
> ---
>  net/ipv4/devinet.c                  |  4 ++++
>  net/ipv4/fib_frontend.c             |  4 ++++
>  net/ipv4/netfilter/ipt_MASQUERADE.c | 12 ++++++++++--
>  3 files changed, 18 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/devinet.c b/net/ipv4/devinet.c
> index 68447109000f..6678bebb82c8 100644
> --- a/net/ipv4/devinet.c
> +++ b/net/ipv4/devinet.c
> @@ -328,6 +328,9 @@ static void __inet_del_ifa(struct in_device *in_dev, struct in_ifaddr **ifap,
>  
>  	ASSERT_RTNL();
>  
> +	if (in_dev->dead)
> +		goto no_promotions;
> +
>  	/* 1. Deleting primary ifaddr forces deletion all secondaries
>  	 * unless alias promotion is set
>  	 **/
> @@ -374,6 +377,7 @@ static void __inet_del_ifa(struct in_device *in_dev, struct in_ifaddr **ifap,
>  			fib_del_ifaddr(ifa, ifa1);
>  	}
>  
> +no_promotions:
>  	/* 2. Unlink it */
>  
>  	*ifap = ifa1->ifa_next;
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index c7539e22868b..4d4952129bc1 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
> @@ -812,6 +812,9 @@ void fib_del_ifaddr(struct in_ifaddr *ifa, struct in_ifaddr *iprim)
>  		subnet = 1;
>  	}
>  
> +	if (in_dev->dead)
> +		goto no_promotions;
> +
>  	/* Deletion is more complicated than add.
>  	 * We should take care of not to delete too much :-)
>  	 *
> @@ -887,6 +890,7 @@ void fib_del_ifaddr(struct in_ifaddr *ifa, struct in_ifaddr *iprim)
>  		}
>  	}
>  
> +no_promotions:
>  	if (!(ok & BRD_OK))
>  		fib_magic(RTM_DELROUTE, RTN_BROADCAST, ifa->ifa_broadcast, 32, prim);
>  	if (subnet && ifa->ifa_prefixlen < 31) {
> diff --git a/net/ipv4/netfilter/ipt_MASQUERADE.c b/net/ipv4/netfilter/ipt_MASQUERADE.c
> index 00352ce0f0de..3bc1c98aa2f0 100644
> --- a/net/ipv4/netfilter/ipt_MASQUERADE.c
> +++ b/net/ipv4/netfilter/ipt_MASQUERADE.c
> @@ -128,10 +128,18 @@ static int masq_inet_event(struct notifier_block *this,
>  			   unsigned long event,
>  			   void *ptr)
>  {
> -	struct net_device *dev = ((struct in_ifaddr *)ptr)->ifa_dev->dev;
> +	struct in_device *idev = ((struct in_ifaddr *)ptr)->ifa_dev;
>  	struct netdev_notifier_info info;
>  
> -	netdev_notifier_info_init(&info, dev);
> +	/* The masq_dev_notifier will catch the case of the device going
> +	 * down.  So if the inetdev is dead and being destroyed we have
> +	 * no work to do.  Otherwise this is an individual address removal
> +	 * and we have to perform the flush.
> +	 */
> +	if (idev->dead)
> +		return NOTIFY_DONE;
> +
> +	netdev_notifier_info_init(&info, idev->dev);
>  	return masq_device_event(this, event, &info);
>  }
>  
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team
>