netdev - Re: [PATCH V2 net-next 1/2] tcp: send in-queue bytes in cmsg upon read

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Mon, 30 Apr 2018 11:38:34 -0400 (EDT)
From:   David Miller <davem@...emloft.net>
To:     soheil.kdev@...il.com
Cc:     netdev@...r.kernel.org, ycheng@...gle.com, ncardwell@...gle.com,
        edumazet@...gle.com, willemb@...gle.com, soheil@...gle.com
Subject: Re: [PATCH V2 net-next 1/2] tcp: send in-queue bytes in cmsg upon
 read

From: Soheil Hassas Yeganeh <soheil.kdev@...il.com>
Date: Fri, 27 Apr 2018 14:57:32 -0400

> Since the socket lock is not held when calculating the size of
> receive queue, TCP_INQ is a hint.  For example, it can overestimate
> the queue size by one byte, if FIN is received.

I think it is even worse than that.

If another application comes in and does a recvmsg() in parallel with
these calculations, you could even report a negative value.

These READ_ONCE() make it look like some of these issues are being
addressed but they are not.

You could freeze the values just by taking sk->sk_lock.slock, but I
don't know if that cost is considered acceptable or not.

Another idea is to sample both values in a loop, similar to a sequence
lock sequence:

again:
	tmp1 = A;
	tmp2 = B;
	barrier();
	tmp3 = A;
	if (tmp1 != tmp3)
		goto again;

But the current state of affairs is not going to work well.