Skip to content

Commit 94731cc

Browse files
LGA1150gregkh
authored andcommitted
ppp: fix race conditions in ppp_fill_forward_path
[ Upstream commit 0417adf ] 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: f6efc67 ("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>
1 parent b6be9f9 commit 94731cc

File tree

1 file changed

+11
-6
lines changed

1 file changed

+11
-6
lines changed

drivers/net/ppp/ppp_generic.c

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include <linux/ppp_channel.h>
3434
#include <linux/ppp-comp.h>
3535
#include <linux/skbuff.h>
36+
#include <linux/rculist.h>
3637
#include <linux/rtnetlink.h>
3738
#include <linux/if_arp.h>
3839
#include <linux/ip.h>
@@ -1613,11 +1614,14 @@ static int ppp_fill_forward_path(struct net_device_path_ctx *ctx,
16131614
if (ppp->flags & SC_MULTILINK)
16141615
return -EOPNOTSUPP;
16151616

1616-
if (list_empty(&ppp->channels))
1617+
pch = list_first_or_null_rcu(&ppp->channels, struct channel, clist);
1618+
if (!pch)
1619+
return -ENODEV;
1620+
1621+
chan = READ_ONCE(pch->chan);
1622+
if (!chan)
16171623
return -ENODEV;
16181624

1619-
pch = list_first_entry(&ppp->channels, struct channel, clist);
1620-
chan = pch->chan;
16211625
if (!chan->ops->fill_forward_path)
16221626
return -EOPNOTSUPP;
16231627

@@ -3000,7 +3004,7 @@ ppp_unregister_channel(struct ppp_channel *chan)
30003004
*/
30013005
down_write(&pch->chan_sem);
30023006
spin_lock_bh(&pch->downl);
3003-
pch->chan = NULL;
3007+
WRITE_ONCE(pch->chan, NULL);
30043008
spin_unlock_bh(&pch->downl);
30053009
up_write(&pch->chan_sem);
30063010
ppp_disconnect_channel(pch);
@@ -3506,7 +3510,7 @@ ppp_connect_channel(struct channel *pch, int unit)
35063510
hdrlen = pch->file.hdrlen + 2; /* for protocol bytes */
35073511
if (hdrlen > ppp->dev->hard_header_len)
35083512
ppp->dev->hard_header_len = hdrlen;
3509-
list_add_tail(&pch->clist, &ppp->channels);
3513+
list_add_tail_rcu(&pch->clist, &ppp->channels);
35103514
++ppp->n_channels;
35113515
pch->ppp = ppp;
35123516
refcount_inc(&ppp->file.refcnt);
@@ -3536,10 +3540,11 @@ ppp_disconnect_channel(struct channel *pch)
35363540
if (ppp) {
35373541
/* remove it from the ppp unit's list */
35383542
ppp_lock(ppp);
3539-
list_del(&pch->clist);
3543+
list_del_rcu(&pch->clist);
35403544
if (--ppp->n_channels == 0)
35413545
wake_up_interruptible(&ppp->file.rwait);
35423546
ppp_unlock(ppp);
3547+
synchronize_net();
35433548
if (refcount_dec_and_test(&ppp->file.refcnt))
35443549
ppp_destroy_interface(ppp);
35453550
err = 0;

0 commit comments

Comments
 (0)