Re: [PATCH net-next] ppp: consolidate RX skb queueing
From: Sebastian Andrzej Siewior
Date: Tue Apr 28 2026 - 02:43:52 EST
On 2026-04-28 10:44:23 [+0800], Qingfang Deng wrote:
> In ppp_input() and ppp_receive_nonmp_frame(), received skbs are queued
> for userspace delivery using the same open-coded pattern:
>
> skb_queue_tail(&pf->rq, skb);
> while (pf->rq.qlen > PPP_MAX_RQLEN &&
> (skb = skb_dequeue(&pf->rq)))
> kfree_skb(skb);
> wake_up_interruptible(&pf->rwait);
>
> This has a potential race: skb_queue_tail() releases the queue lock,
> then qlen is read locklessly before skb_dequeue() re-acquires it.
> Another CPU enqueueing concurrently could cause the length check to see
> stale data. This race is benign, as it only causes extra skbs to be
> freed in the worst case.
That is not that bad. You could use skb_queue_len_lockless() to make it
more obvious. However, if thread A enqueues packets and is below the
limit and wakes the reader, it could enqueue more and which point it
will check the limit again. I don't see a problem except that the reader
may get more packets before the queue is trimmed. Again, not an issue.
It is only here to prevent a large amount of packets if userland does
not read the queue for some reason.
Merging the two instances into one function would be nice but there is
no need to complicate things.
Sebastian