[PATCH net] ipv4: igmp: Allow removing groups from a removed interface

[PATCH net] ipv4: igmp: Allow removing groups from a removed interface

[Andrew Lunn]

When a multicast group is joined on a socket, a struct ip_mc_socklist
is appended to the sockets mc_list containing information about the
joined group.

If the interface is hot unplugged, this entry becomes stale. Prior to
52ad353a5344f "(igmp: fix the problem when mc leave group") it was
possible to remove the stale entry by performing a IP_DROP_MEMBERSHIP,
passing either the old ifindex or ip address on the
interface. However, this fix enforces that the interface must still
exist. Thus with time, the number of stale entries grows, until
sysctl_igmp_max_memberships is reached and then it is not possible to
join any more groups.

The 52ad353a5344f fixes an issue where a IP_DROP_MEMBERSHIP is
performed without specifying the interface, either by ifindex or ip
address. However here we do supply one of these. So loosen the
restriction on device existence to only apply when the interface has
not been specified. This then restores the ability to clean up the
stale entries.

Signed-off-by: Andrew Lunn <andrew@lunn.ch>
Fixes: 52ad353a5344f "(igmp: fix the problem when mc leave group")
---
 net/ipv4/igmp.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 6baf36e11808..e31dd1f0a55e 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -2126,7 +2126,7 @@ int ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr)
 	ASSERT_RTNL();
 
 	in_dev = ip_mc_find_dev(net, imr);
-	if (!in_dev) {
+	if (!imr->imr_ifindex && !imr->imr_address.s_addr && !in_dev) {
 		ret = -ENODEV;
 		goto out;
 	}
-- 
2.6.2

Re: [PATCH net] ipv4: igmp: Allow removing groups from a removed interface

[David Miller]

From: Andrew Lunn <andrew@lunn.ch>
Date: Wed, 25 Nov 2015 21:15:36 +0100

> @@ -2126,7 +2126,7 @@ int ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr)
>  	ASSERT_RTNL();
>  
>  	in_dev = ip_mc_find_dev(net, imr);
> -	if (!in_dev) {
> +	if (!imr->imr_ifindex && !imr->imr_address.s_addr && !in_dev) {
>  		ret = -ENODEV;
>  		goto out;
>  	}

Now, ip_mc_dec_group() below can take a NULL pointer dereference.  One example
is if imr_ifindex is specified and the lookup returns NULL in ip_mc_find_dev().

This is so rediculously complicated, just looking at this code breaks something.

Re: [PATCH net] ipv4: igmp: Allow removing groups from a removed interface

[Andrew Lunn]

On Mon, Nov 30, 2015 at 11:01:48AM -0500, David Miller wrote:
> From: Andrew Lunn <andrew@lunn.ch>
> Date: Wed, 25 Nov 2015 21:15:36 +0100
> 
> > @@ -2126,7 +2126,7 @@ int ip_mc_leave_group(struct sock *sk, struct ip_mreqn *imr)
> >  	ASSERT_RTNL();
> >  
> >  	in_dev = ip_mc_find_dev(net, imr);
> > -	if (!in_dev) {
> > +	if (!imr->imr_ifindex && !imr->imr_address.s_addr && !in_dev) {
> >  		ret = -ENODEV;
> >  		goto out;
> >  	}
> 
> Now, ip_mc_dec_group() below can take a NULL pointer dereference.  One example
> is if imr_ifindex is specified and the lookup returns NULL in ip_mc_find_dev().

Agreed. Earlier code had an if (in_dev) before the call to
ip_mc_dec_group(). It got removed along the way and now needs adding
back. A v2 patch will follow soon.
 
> This is so rediculously complicated, just looking at this code breaks something.

Yep. I think part of the problem comes from the code being designed
before interfaces were hot plugable.