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: 1da177e4c3f4 ("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>

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index ef54e0a0462a175bb989db8dda895e9ae755965f..cacc4c3a37d2cda0aea2e176e5d0638f959e18a3 100644 (file)
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c
@@ -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;