Re: [PATCH] net: skb content must be visible for lockless skb_peek() and its variations
From: Paolo Abeni <pabeni@redhat.com>
Date: 2022-08-01 06:52:25
On Sun, 2022-07-31 at 23:39 +0300, Kirill Tkhai wrote:
From: Kirill Tkhai <redacted> Currently, there are no barriers, and skb->xxx update may become invisible on cpu2. In the below example var2 may point to intial_val0 instead of expected var1: [cpu1] [cpu2] skb->xxx = initial_val0; ... skb->xxx = var1; skb = READ_ONCE(prev_skb->next); <no barrier> <no barrier> WRITE_ONCE(prev_skb->next, skb); var2 = skb->xxx; This patch adds barriers and fixes the problem. Note, that __skb_peek() is not patched, since it's a lowlevel function, and a caller has to understand the things it does (and also __skb_peek() is used under queue lock in some places). Signed-off-by: Kirill Tkhai <redacted> --- Hi, David, Eric and other developers, picking unix sockets code I found this problem,
Could you please report exactly how/where the problem maifests (e.g. the involved call paths/time sequence)?
and for me it looks like it exists. If there are arguments that everything is OK and it's expected, please, explain.
I don't see why such barriers are needed for the locked peek/tail variants, as the spin_lock pair implies a full memory barrier. Cheers, Paolo