ppp: annotate data races in ppp_generic

Several fields in struct ppp can be read or updated concurrently
from multiple CPUs without synchronization, causing data races:

1. ppp->mru is read concurrently in ppp_receive_nonmp_frame() while
   being updated via PPPIOCSMRU ioctl. Protect ppp->mru updates in
   PPPIOCSMRU with ppp_recv_lock(ppp).

2. PPPIOCGFLAGS reads ppp->flags, ppp->xstate, and ppp->rstate
   unlocked. Wrap the read in ppp_lock(ppp) to get a consistent
   snapshot.

3. ppp->debug is updated via PPPIOCSDEBUG and read concurrently on
   fast paths. Annotate reads with READ_ONCE() and writes with
   WRITE_ONCE().

4. ppp->last_xmit and ppp->last_recv are updated on TX/RX data paths
   and read via PPPIOCGIDLE32 / PPPIOCGIDLE64 ioctls. Annotate with
   WRITE_ONCE() / READ_ONCE() and use max() to handle jiffies
   subtraction.

5. ppp->npmode[] is updated via PPPIOCSNPMODE and read on TX/RX
   paths. Annotate with WRITE_ONCE() / READ_ONCE().

Fixes: 1da177e4c3 ("Linux-2.6.12-rc2")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reviewed-by: Qingfang Deng <qingfang.deng@linux.dev>
Link: https://patch.msgid.link/20260722101605.2868548-1-edumazet@google.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
This commit is contained in:
Eric Dumazet 2026-07-22 10:16:05 +00:00 committed by Jakub Kicinski
parent 853e164c2b
commit 543adf0721

View File

@ -810,7 +810,9 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
case PPPIOCSMRU:
if (get_user(val, p))
break;
ppp_recv_lock(ppp);
ppp->mru = val;
ppp_recv_unlock(ppp);
err = 0;
break;
@ -831,7 +833,9 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
break;
case PPPIOCGFLAGS:
ppp_lock(ppp);
val = ppp->flags | ppp->xstate | ppp->rstate;
ppp_unlock(ppp);
if (put_user(val, p))
break;
err = 0;
@ -855,7 +859,7 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
case PPPIOCSDEBUG:
if (get_user(val, p))
break;
ppp->debug = val;
WRITE_ONCE(ppp->debug, val);
err = 0;
break;
@ -866,16 +870,16 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
break;
case PPPIOCGIDLE32:
idle32.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
idle32.recv_idle = (jiffies - ppp->last_recv) / HZ;
if (copy_to_user(argp, &idle32, sizeof(idle32)))
idle32.xmit_idle = max(0L, (long)(jiffies - READ_ONCE(ppp->last_xmit))) / HZ;
idle32.recv_idle = max(0L, (long)(jiffies - READ_ONCE(ppp->last_recv))) / HZ;
if (copy_to_user(argp, &idle32, sizeof(idle32)))
break;
err = 0;
break;
case PPPIOCGIDLE64:
idle64.xmit_idle = (jiffies - ppp->last_xmit) / HZ;
idle64.recv_idle = (jiffies - ppp->last_recv) / HZ;
idle64.xmit_idle = max(0L, (long)(jiffies - READ_ONCE(ppp->last_xmit))) / HZ;
idle64.recv_idle = max(0L, (long)(jiffies - READ_ONCE(ppp->last_recv))) / HZ;
if (copy_to_user(argp, &idle64, sizeof(idle64)))
break;
err = 0;
@ -916,7 +920,7 @@ static long ppp_ioctl(struct file *file, unsigned int cmd, unsigned long arg)
if (copy_to_user(argp, &npi, sizeof(npi)))
break;
} else {
ppp->npmode[i] = npi.mode;
WRITE_ONCE(ppp->npmode[i], npi.mode);
/* we may be able to transmit more packets now (??) */
netif_wake_queue(ppp->dev);
}
@ -1454,7 +1458,7 @@ ppp_start_xmit(struct sk_buff *skb, struct net_device *dev)
goto outf;
/* Drop, accept or reject the packet */
switch (ppp->npmode[npi]) {
switch (READ_ONCE(ppp->npmode[npi])) {
case NPMODE_PASS:
break;
case NPMODE_QUEUE:
@ -1790,7 +1794,7 @@ ppp_prepare_tx_skb(struct ppp *ppp, struct sk_buff **pskb)
*(__be16 *)skb_push(skb, 2) = htons(PPP_FILTER_OUTBOUND_TAG);
if (ppp->pass_filter &&
bpf_prog_run(ppp->pass_filter, skb) == 0) {
if (ppp->debug & 1)
if (READ_ONCE(ppp->debug) & 1)
netdev_printk(KERN_DEBUG, ppp->dev,
"PPP: outbound frame "
"not passed\n");
@ -1800,11 +1804,11 @@ ppp_prepare_tx_skb(struct ppp *ppp, struct sk_buff **pskb)
/* if this packet passes the active filter, record the time */
if (!(ppp->active_filter &&
bpf_prog_run(ppp->active_filter, skb) == 0))
ppp->last_xmit = jiffies;
WRITE_ONCE(ppp->last_xmit, jiffies);
skb_pull(skb, 2);
#else
/* for data packets, record the time */
ppp->last_xmit = jiffies;
WRITE_ONCE(ppp->last_xmit, jiffies);
#endif /* CONFIG_PPP_FILTER */
}
@ -2154,7 +2158,7 @@ static int ppp_mp_explode(struct ppp *ppp, struct sk_buff *skb)
noskb:
spin_unlock(&pch->downl);
err_linearize:
if (ppp->debug & 1)
if (READ_ONCE(ppp->debug) & 1)
netdev_err(ppp->dev, "PPP: no memory (fragment)\n");
DEV_STATS_INC(ppp->dev, tx_errors);
++ppp->nxseq;
@ -2502,7 +2506,7 @@ ppp_receive_nonmp_frame(struct ppp *ppp, struct sk_buff *skb)
*(__be16 *)skb_push(skb, 2) = htons(PPP_FILTER_INBOUND_TAG);
if (ppp->pass_filter &&
bpf_prog_run(ppp->pass_filter, skb) == 0) {
if (ppp->debug & 1)
if (READ_ONCE(ppp->debug) & 1)
netdev_printk(KERN_DEBUG, ppp->dev,
"PPP: inbound frame "
"not passed\n");
@ -2511,14 +2515,14 @@ ppp_receive_nonmp_frame(struct ppp *ppp, struct sk_buff *skb)
}
if (!(ppp->active_filter &&
bpf_prog_run(ppp->active_filter, skb) == 0))
ppp->last_recv = jiffies;
WRITE_ONCE(ppp->last_recv, jiffies);
__skb_pull(skb, 2);
} else
#endif /* CONFIG_PPP_FILTER */
ppp->last_recv = jiffies;
WRITE_ONCE(ppp->last_recv, jiffies);
if ((ppp->dev->flags & IFF_UP) == 0 ||
ppp->npmode[npi] != NPMODE_PASS) {
READ_ONCE(ppp->npmode[npi]) != NPMODE_PASS) {
kfree_skb(skb);
} else {
/* chop off protocol */
@ -2771,7 +2775,7 @@ ppp_mp_reconstruct(struct ppp *ppp)
seq = seq_before(minseq, PPP_MP_CB(p)->sequence)?
minseq + 1: PPP_MP_CB(p)->sequence;
if (ppp->debug & 1)
if (READ_ONCE(ppp->debug) & 1)
netdev_printk(KERN_DEBUG, ppp->dev,
"lost frag %u..%u\n",
oldseq, seq-1);
@ -2820,7 +2824,7 @@ ppp_mp_reconstruct(struct ppp *ppp)
struct sk_buff *tmp2;
skb_queue_reverse_walk_from_safe(list, p, tmp2) {
if (ppp->debug & 1)
if (READ_ONCE(ppp->debug) & 1)
netdev_printk(KERN_DEBUG, ppp->dev,
"discarding frag %u\n",
PPP_MP_CB(p)->sequence);
@ -2842,7 +2846,7 @@ ppp_mp_reconstruct(struct ppp *ppp)
skb_queue_walk_safe(list, p, tmp) {
if (p == head)
break;
if (ppp->debug & 1)
if (READ_ONCE(ppp->debug) & 1)
netdev_printk(KERN_DEBUG, ppp->dev,
"discarding frag %u\n",
PPP_MP_CB(p)->sequence);
@ -2850,7 +2854,7 @@ ppp_mp_reconstruct(struct ppp *ppp)
kfree_skb(p);
}
if (ppp->debug & 1)
if (READ_ONCE(ppp->debug) & 1)
netdev_printk(KERN_DEBUG, ppp->dev,
" missed pkts %u..%u\n",
ppp->nextseq,
@ -3161,7 +3165,8 @@ ppp_ccp_peek(struct ppp *ppp, struct sk_buff *skb, int inbound)
if (!ppp->rc_state)
break;
if (ppp->rcomp->decomp_init(ppp->rc_state, dp, len,
ppp->file.index, 0, ppp->mru, ppp->debug)) {
ppp->file.index, 0, ppp->mru,
READ_ONCE(ppp->debug))) {
ppp->rstate |= SC_DECOMP_RUN;
ppp->rstate &= ~(SC_DC_ERROR | SC_DC_FERROR);
}
@ -3170,7 +3175,8 @@ ppp_ccp_peek(struct ppp *ppp, struct sk_buff *skb, int inbound)
if (!ppp->xc_state)
break;
if (ppp->xcomp->comp_init(ppp->xc_state, dp, len,
ppp->file.index, 0, ppp->debug))
ppp->file.index, 0,
READ_ONCE(ppp->debug)))
ppp->xstate |= SC_COMP_RUN;
}
break;