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 <[email protected]> wrote:
>> Hi,
>>
>> On 2026/7/22 18:16, Eric Dumazet wrote:
>>> 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 <[email protected]>