On Mon, Apr 30, 2018 at 12:10 PM, David Miller <da...@davemloft.net> wrote:
> From: Eric Dumazet <eric.duma...@gmail.com>
> Date: Mon, 30 Apr 2018 09:01:47 -0700
>
>> TCP sockets are read by a single thread really (or synchronized
>> threads), or garbage is ensured, regardless of how the kernel
>> ensures locking while reporting "queue length"
>
> Whatever applications "typically do", we should never return
> garbage, and that is what this code allowing to happen.
>
> Everything else in recvmsg() operates on state under the proper socket
> lock, to ensure consistency.
>
> The only reason we are releasing the socket lock first it to make sure
> the backlog is processed and we have the most update information
> available.
>
> It seems like one is striving for correctness and better accuracy, no?
> :-)
>
> Look, this can be fixed really simply. And if you are worried about
> unbounded loops if two apps maliciously do recvmsg() in parallel,
> then don't even loop, just fallback to full socket locking and make
> the "non-typical" application pay the price:
>
> tmp1 = A;
> tmp2 = B;
> barrier();
> tmp3 = A;
> if (unlikely(tmp1 != tmp3)) {
> lock_sock(sk);
> tmp1 = A;
> tmp2 = B;
> release_sock(sk);
> }
>
> I'm seriously not applying the patch as-is, sorry. This issue
> must be addressed somehow.
Thank you David for the suggestion. Sure, I'll send a V3 with what you
suggested above.
Thanks,
Soheil
