[PATCH v1 net 0/2] ipmr/ip6mr: Fix mr_mfc.unres.unresolved corruption.

[PATCH v1 net 0/2] ipmr/ip6mr: Fix mr_mfc.unres.unresolved corruption.

[Kuniyuki Iwashima]

ipmr_cache_resolve() / ip6mr_cache_resolve() dequeues
skb from mr_mfc.unres.unresolved with no protection.

Patch 1 / 2 replace __skb_dequeue() with skb_dequeue()
in ipmr_cache_resolve() / ip6mr_cache_resolve().


Kuniyuki Iwashima (2):
  ipmr: Fix mr_mfc.unres.unresolved corruption in ipmr_cache_resolve().
  ip6mr: Fix mr_mfc.unres.unresolved corruption in 
    ip6mr_cache_resolve().

 net/ipv4/ipmr.c  | 2 +-
 net/ipv6/ip6mr.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

-- 
2.53.0.371.g1d285c8824-goog

[PATCH v1 net 1/2] ipmr: Fix mr_mfc.unres.unresolved corruption in ipmr_cache_resolve().

[Kuniyuki Iwashima]

mr_mfc.unres.unresolved is filled by skb_queue_tail() under
spin_lock_bh(&mfc_unres_lock) in ipmr_cache_unresolved().

ipmr_cache_resolve() is called from ipmr_mfc_add() after
releasing the spinlock, so nothing protects the queue.

Let's use skb_dequeue() instead.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/ipmr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
index 131382c388e9..62fe54cf7705 100644
--- a/net/ipv4/ipmr.c
+++ b/net/ipv4/ipmr.c
@@ -1002,7 +1002,7 @@ static void ipmr_cache_resolve(struct net *net, struct mr_table *mrt,
 	struct nlmsgerr *e;
 
 	/* Play the pending entries through our router */
-	while ((skb = __skb_dequeue(&uc->_c.mfc_un.unres.unresolved))) {
+	while ((skb = skb_dequeue(&uc->_c.mfc_un.unres.unresolved))) {
 		if (ip_hdr(skb)->version == 0) {
 			struct nlmsghdr *nlh = skb_pull(skb,
 							sizeof(struct iphdr));
-- 
2.53.0.371.g1d285c8824-goog

[PATCH v1 net 2/2] ip6mr: Fix mr_mfc.unres.unresolved corruption in ip6mr_cache_resolve().

[Kuniyuki Iwashima]

mr_mfc.unres.unresolved is filled by skb_queue_tail() under
spin_lock_bh(&mfc_unres_lock) in ip6mr_cache_unresolved().

ip6mr_cache_resolve() is called from ip6mr_mfc_add() after
releasing the spinlock, so nothing protects the queue.

Let's use skb_dequeue() instead.

Fixes: 7bc570c8b4f7 ("[IPV6] MROUTE: Support multicast forwarding.")
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv6/ip6mr.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv6/ip6mr.c b/net/ipv6/ip6mr.c
index e047a4680ab0..97c406e58b39 100644
--- a/net/ipv6/ip6mr.c
+++ b/net/ipv6/ip6mr.c
@@ -1031,7 +1031,7 @@ static void ip6mr_cache_resolve(struct net *net, struct mr_table *mrt,
 	 *	Play the pending entries through our router
 	 */
 
-	while ((skb = __skb_dequeue(&uc->_c.mfc_un.unres.unresolved))) {
+	while ((skb = skb_dequeue(&uc->_c.mfc_un.unres.unresolved))) {
 		if (ipv6_hdr(skb)->version == 0) {
 			struct nlmsghdr *nlh = skb_pull(skb,
 							sizeof(struct ipv6hdr));
-- 
2.53.0.371.g1d285c8824-goog

Re: [PATCH v1 net 1/2] ipmr: Fix mr_mfc.unres.unresolved corruption in ipmr_cache_resolve().

[Eric Dumazet]

On Sun, Feb 22, 2026 at 8:50 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
>
> mr_mfc.unres.unresolved is filled by skb_queue_tail() under
> spin_lock_bh(&mfc_unres_lock) in ipmr_cache_unresolved().
>
> ipmr_cache_resolve() is called from ipmr_mfc_add() after
> releasing the spinlock, so nothing protects the queue.
>
> Let's use skb_dequeue() instead.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> ---
>  net/ipv4/ipmr.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> index 131382c388e9..62fe54cf7705 100644
> --- a/net/ipv4/ipmr.c
> +++ b/net/ipv4/ipmr.c
> @@ -1002,7 +1002,7 @@ static void ipmr_cache_resolve(struct net *net, struct mr_table *mrt,
>         struct nlmsgerr *e;
>
>         /* Play the pending entries through our router */
> -       while ((skb = __skb_dequeue(&uc->_c.mfc_un.unres.unresolved))) {
> +       while ((skb = skb_dequeue(&uc->_c.mfc_un.unres.unresolved))) {
>                 if (ip_hdr(skb)->version == 0) {
>                         struct nlmsghdr *nlh = skb_pull(skb,
>                                                         sizeof(struct iphdr));
> --
> 2.53.0.371.g1d285c8824-goog

Are you sure this is needed ?

ipmr_mfc_add()
..
    if (found) {
      ipmr_cache_resolve(net, mrt, uc, c);
     ipmr_cache_resolve(net, mrt, uc, c); // This would still be racy
  }

My understanding of this code is that ipmr_mfc_add() removed uc from
&mrt->mfc_unres_queue
under the mfc_unres_lock spinlock protection.

Re: [PATCH v1 net 1/2] ipmr: Fix mr_mfc.unres.unresolved corruption in ipmr_cache_resolve().

[Kuniyuki Iwashima]

On Mon, Feb 23, 2026 at 2:15 AM Eric Dumazet <edumazet@google.com> wrote:
>
> On Sun, Feb 22, 2026 at 8:50 PM Kuniyuki Iwashima <kuniyu@google.com> wrote:
> >
> > mr_mfc.unres.unresolved is filled by skb_queue_tail() under
> > spin_lock_bh(&mfc_unres_lock) in ipmr_cache_unresolved().
> >
> > ipmr_cache_resolve() is called from ipmr_mfc_add() after
> > releasing the spinlock, so nothing protects the queue.
> >
> > Let's use skb_dequeue() instead.
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
> > ---
> >  net/ipv4/ipmr.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/net/ipv4/ipmr.c b/net/ipv4/ipmr.c
> > index 131382c388e9..62fe54cf7705 100644
> > --- a/net/ipv4/ipmr.c
> > +++ b/net/ipv4/ipmr.c
> > @@ -1002,7 +1002,7 @@ static void ipmr_cache_resolve(struct net *net, struct mr_table *mrt,
> >         struct nlmsgerr *e;
> >
> >         /* Play the pending entries through our router */
> > -       while ((skb = __skb_dequeue(&uc->_c.mfc_un.unres.unresolved))) {
> > +       while ((skb = skb_dequeue(&uc->_c.mfc_un.unres.unresolved))) {
> >                 if (ip_hdr(skb)->version == 0) {
> >                         struct nlmsghdr *nlh = skb_pull(skb,
> >                                                         sizeof(struct iphdr));
> > --
> > 2.53.0.371.g1d285c8824-goog
>
> Are you sure this is needed ?
>
> ipmr_mfc_add()
> ..
>     if (found) {
>       ipmr_cache_resolve(net, mrt, uc, c);
>      ipmr_cache_resolve(net, mrt, uc, c); // This would still be racy
>   }
>
> My understanding of this code is that ipmr_mfc_add() removed uc from
> &mrt->mfc_unres_queue
> under the mfc_unres_lock spinlock protection.

Ah you are right, maybe I missed the list_del() somehow.

Thank you!