Re: [PATCH net 3/3] ipv4: igmp: Fix potential memory leak in igmp_mod_timer()

[Ido Schimmel <idosch@nvidia.com>]

On Sat, Jul 04, 2026 at 07:43:46PM +0000, Eric Dumazet wrote:
> When a timer is deleted and not re-armed in igmp_mod_timer(), the code
> currently decrements the reference counter of the multicast list entry
> @im using refcount_dec(&im->refcnt).
> 
> However, igmp_mod_timer() can be called from the RCU reader path (e.g., in
> igmp_heard_query() via for_each_pmc_rcu()). If the group im was
> concurrently removed from the list by ip_mc_dec_group(), its reference count
> might have already been decremented to 1.
> 
> In this case, timer_delete() succeeds, and refcount_dec() decrements
> the refcount from 1 to 0. Since refcount_dec() does not free the object
> when it hits 0 (unlike ip_ma_put()), the im structure is leaked.
> 
> Fix this by using ip_ma_put(im) instead of refcount_dec(&im->refcnt).
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> ---
>  net/ipv4/igmp.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
> index f5f9763895641bf86bfcf9fd7fd7b06012fa4ece..2170b33ba147ce4990e3ee71ba4868e8696b00cb 100644
> --- a/net/ipv4/igmp.c
> +++ b/net/ipv4/igmp.c
> @@ -266,6 +266,8 @@ static void igmp_ifc_start_timer(struct in_device *in_dev, int delay)
>  
>  static void igmp_mod_timer(struct ip_mc_list *im, int max_delay)
>  {
> +	bool put = false;
> +
>  	spin_lock_bh(&im->lock);
>  	im->unsolicit_count = 0;
>  	if (timer_delete(&im->timer)) {
> @@ -275,10 +277,13 @@ static void igmp_mod_timer(struct ip_mc_list *im, int max_delay)
>  			spin_unlock_bh(&im->lock);
>  			return;
>  		}
> -		refcount_dec(&im->refcnt);
> +		put = true;
>  	}
>  	igmp_start_timer(im, max_delay);
>  	spin_unlock_bh(&im->lock);
> +
> +	if (put)
> +		ip_ma_put(im);
>  }

Don't we also need something similar in igmp_stop_timer() [1]? It can
also be called from an RCU reader (i.e., igmp_rcv() ->
igmp_heard_report() -> igmp_stop_timer()).

[1]
diff --git a/net/ipv4/igmp.c b/net/ipv4/igmp.c
index 8e2b60dcc183..d08331b44519 100644
--- a/net/ipv4/igmp.c
+++ b/net/ipv4/igmp.c
@@ -217,13 +217,18 @@ static void ip_sf_list_clear_all(struct ip_sf_list *psf)
 
 static void igmp_stop_timer(struct ip_mc_list *im)
 {
+	bool put = false;
+
 	spin_lock_bh(&im->lock);
 	if (timer_delete(&im->timer))
-		refcount_dec(&im->refcnt);
+		put = true;
 	WRITE_ONCE(im->tm_running, 0);
 	WRITE_ONCE(im->reporter, 0);
 	im->unsolicit_count = 0;
 	spin_unlock_bh(&im->lock);
+
+	if (put)
+		ip_ma_put(im);
 }
 
 /* It must be called with locked im->lock */