ACK: [Impish/OEM-5.14 1/1] ipv6: fix skb drops in igmp6_event_query() and igmp6_event_report()

Kleber Souza kleber.souza at canonical.com
Tue Mar 15 13:40:53 UTC 2022


On 15.03.22 13:10, Thadeu Lima de Souza Cascardo wrote:
> From: Eric Dumazet <edumazet at google.com>
> 
> While investigating on why a synchronize_net() has been added recently
> in ipv6_mc_down(), I found that igmp6_event_query() and igmp6_event_report()
> might drop skbs in some cases.
> 
> Discussion about removing synchronize_net() from ipv6_mc_down()
> will happen in a different thread.
> 
> Fixes: f185de28d9ae ("mld: add new workqueues for process mld events")
> Signed-off-by: Eric Dumazet <edumazet at google.com>
> Cc: Taehee Yoo <ap420073 at gmail.com>
> Cc: Cong Wang <xiyou.wangcong at gmail.com>
> Cc: David Ahern <dsahern at kernel.org>
> Link: https://lore.kernel.org/r/20220303173728.937869-1-eric.dumazet@gmail.com
> Signed-off-by: Jakub Kicinski <kuba at kernel.org>
> (cherry picked from commit 2d3916f3189172d5c69d33065c3c21119fe539fc)
> CVE-2022-0742
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>

Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>

Thanks

> ---
>   include/net/ndisc.h |  4 ++--
>   net/ipv6/mcast.c    | 32 ++++++++++++--------------------
>   2 files changed, 14 insertions(+), 22 deletions(-)
> 
> diff --git a/include/net/ndisc.h b/include/net/ndisc.h
> index 38e4094960ce..e97ef508664f 100644
> --- a/include/net/ndisc.h
> +++ b/include/net/ndisc.h
> @@ -487,9 +487,9 @@ int igmp6_late_init(void);
>   void igmp6_cleanup(void);
>   void igmp6_late_cleanup(void);
>   
> -int igmp6_event_query(struct sk_buff *skb);
> +void igmp6_event_query(struct sk_buff *skb);
>   
> -int igmp6_event_report(struct sk_buff *skb);
> +void igmp6_event_report(struct sk_buff *skb);
>   
>   
>   #ifdef CONFIG_SYSCTL
> diff --git a/net/ipv6/mcast.c b/net/ipv6/mcast.c
> index d36ef9d25e73..e51b9c5d1ddf 100644
> --- a/net/ipv6/mcast.c
> +++ b/net/ipv6/mcast.c
> @@ -1366,27 +1366,23 @@ static int mld_process_v2(struct inet6_dev *idev, struct mld2_query *mld,
>   }
>   
>   /* called with rcu_read_lock() */
> -int igmp6_event_query(struct sk_buff *skb)
> +void igmp6_event_query(struct sk_buff *skb)
>   {
>   	struct inet6_dev *idev = __in6_dev_get(skb->dev);
>   
> -	if (!idev)
> -		return -EINVAL;
> -
> -	if (idev->dead) {
> -		kfree_skb(skb);
> -		return -ENODEV;
> -	}
> +	if (!idev || idev->dead)
> +		goto out;
>   
>   	spin_lock_bh(&idev->mc_query_lock);
>   	if (skb_queue_len(&idev->mc_query_queue) < MLD_MAX_SKBS) {
>   		__skb_queue_tail(&idev->mc_query_queue, skb);
>   		if (!mod_delayed_work(mld_wq, &idev->mc_query_work, 0))
>   			in6_dev_hold(idev);
> +		skb = NULL;
>   	}
>   	spin_unlock_bh(&idev->mc_query_lock);
> -
> -	return 0;
> +out:
> +	kfree_skb(skb);
>   }
>   
>   static void __mld_query_work(struct sk_buff *skb)
> @@ -1539,27 +1535,23 @@ static void mld_query_work(struct work_struct *work)
>   }
>   
>   /* called with rcu_read_lock() */
> -int igmp6_event_report(struct sk_buff *skb)
> +void igmp6_event_report(struct sk_buff *skb)
>   {
>   	struct inet6_dev *idev = __in6_dev_get(skb->dev);
>   
> -	if (!idev)
> -		return -EINVAL;
> -
> -	if (idev->dead) {
> -		kfree_skb(skb);
> -		return -ENODEV;
> -	}
> +	if (!idev || idev->dead)
> +		goto out;
>   
>   	spin_lock_bh(&idev->mc_report_lock);
>   	if (skb_queue_len(&idev->mc_report_queue) < MLD_MAX_SKBS) {
>   		__skb_queue_tail(&idev->mc_report_queue, skb);
>   		if (!mod_delayed_work(mld_wq, &idev->mc_report_work, 0))
>   			in6_dev_hold(idev);
> +		skb = NULL;
>   	}
>   	spin_unlock_bh(&idev->mc_report_lock);
> -
> -	return 0;
> +out:
> +	kfree_skb(skb);
>   }
>   
>   static void __mld_report_work(struct sk_buff *skb)




More information about the kernel-team mailing list