ppp: fix race conditions in ppp_fill_forward_path

[ Upstream commit 0417adf367a0af11adf7ace849af4638cfb573f7 ]

ppp_fill_forward_path() has two race conditions:

1. The ppp->channels list can change between list_empty() and
   list_first_entry(), as ppp_lock() is not held. If the only channel
   is deleted in ppp_disconnect_channel(), list_first_entry() may
   access an empty head or a freed entry, and trigger a panic.

2. pch->chan can be NULL. When ppp_unregister_channel() is called,
   pch->chan is set to NULL before pch is removed from ppp->channels.

Fix these by using a lockless RCU approach:
- Use list_first_or_null_rcu() to safely test and access the first list
  entry.
- Convert list modifications on ppp->channels to their RCU variants and
  add synchronize_net() after removal.
- Check for a NULL pch->chan before dereferencing it.

Fixes: f6efc675c9dd ("net: ppp: resolve forwarding path for bridge pppoe devices")
Signed-off-by: Qingfang Deng <dqfext@gmail.com>
Link: https://patch.msgid.link/20250814012559.3705-2-dqfext@gmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index def84e87e05b2e9daa7a5717d20b58854d74dbf1..5e7672d2022c920af7f0014d641c8b6a7b43ca30 100644 (file)
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -33,6 +33,7 @@
 #include <linux/ppp_channel.h>
 #include <linux/ppp-comp.h>
 #include <linux/skbuff.h>
+#include <linux/rculist.h>
 #include <linux/rtnetlink.h>
 #include <linux/if_arp.h>
 #include <linux/ip.h>
@@ -1612,11 +1613,14 @@ static int ppp_fill_forward_path(struct net_device_path_ctx *ctx,
        if (ppp->flags & SC_MULTILINK)
                return -EOPNOTSUPP;
 
-       if (list_empty(&ppp->channels))
+       pch = list_first_or_null_rcu(&ppp->channels, struct channel, clist);
+       if (!pch)
+               return -ENODEV;
+
+       chan = READ_ONCE(pch->chan);
+       if (!chan)
                return -ENODEV;
 
-       pch = list_first_entry(&ppp->channels, struct channel, clist);
-       chan = pch->chan;
        if (!chan->ops->fill_forward_path)
                return -EOPNOTSUPP;
 
@@ -2999,7 +3003,7 @@ ppp_unregister_channel(struct ppp_channel *chan)
         */
        down_write(&pch->chan_sem);
        spin_lock_bh(&pch->downl);
-       pch->chan = NULL;
+       WRITE_ONCE(pch->chan, NULL);
        spin_unlock_bh(&pch->downl);
        up_write(&pch->chan_sem);
        ppp_disconnect_channel(pch);
@@ -3509,7 +3513,7 @@ ppp_connect_channel(struct channel *pch, int unit)
        hdrlen = pch->file.hdrlen + 2;  /* for protocol bytes */
        if (hdrlen > ppp->dev->hard_header_len)
                ppp->dev->hard_header_len = hdrlen;
-       list_add_tail(&pch->clist, &ppp->channels);
+       list_add_tail_rcu(&pch->clist, &ppp->channels);
        ++ppp->n_channels;
        pch->ppp = ppp;
        refcount_inc(&ppp->file.refcnt);
@@ -3539,10 +3543,11 @@ ppp_disconnect_channel(struct channel *pch)
        if (ppp) {
                /* remove it from the ppp unit's list */
                ppp_lock(ppp);
-               list_del(&pch->clist);
+               list_del_rcu(&pch->clist);
                if (--ppp->n_channels == 0)
                        wake_up_interruptible(&ppp->file.rwait);
                ppp_unlock(ppp);
+               synchronize_net();
                if (refcount_dec_and_test(&ppp->file.refcnt))
                        ppp_destroy_interface(ppp);
                err = 0;