Re: [PATCH net] ppp: annotate data races in ppp_generic

Hi,

On 2026/7/23 14:48, Eric Dumazet wrote:

On Thu, Jul 23, 2026 at 8:24 AM Qingfang Deng [off-list ref] wrote:

quoted

Hi,

On 2026/7/22 18:16, Eric Dumazet wrote:

quoted

diff --git a/drivers/net/ppp/ppp_generic.c b/drivers/net/ppp/ppp_generic.c
index ef54e0a0462a175bb989db8dda895e9ae755965f..cacc4c3a37d2cda0aea2e176e5d0638f959e18a3 100644
--- a/drivers/net/ppp/ppp_generic.c
+++ b/drivers/net/ppp/ppp_generic.c

@@ -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;

In the original code, the difference will wrap around on a 32-bit kernel
with HZ=1000 if the session sits idle for 4,294,967.295 seconds
(approximately 49.7 days). With this change, as the difference is cast
to a long, it will be negative after 2,147,483.647 seconds
(approximately 24.9 days) and then clamped to zero by max(), making the
situation worse.

I do not think the code was ready to handle long idle sessions to begin with.

Can you elaborate on the 'worse' part, because I do not get it,

We perform a max(0L, delta) quite often in the kernel, in lockless contexts.

delta = jiffies - ppp->last_recv ; can be ~0UL if another cpu was able
to update last_recv to (jiffies+1).

We prefer in this case returning 0 instead of ~infinity.

Now I get it. A CPU may see an up-to-date `last_recv` from another CPU 
but a stale `jiffies`, as the order of reading the two variables is 
undefined (sequence points and memory ordering). So:

Reviewed-by: Qingfang Deng <redacted>