netdev - [net PATCH] net: tls, correctly account for copied bytes with multiple sk

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-next>] [day] [month] [year] [list]

Message-ID: <156023370286.5966.10762957456071886488.stgit@ubuntu-kvm1>
Date:   Tue, 11 Jun 2019 06:15:03 +0000
From:   John Fastabend <john.fastabend@...il.com>
To:     steinar+kernel@...derson.no, daniel@...earbox.net, andre@...t.net
Cc:     netdev@...r.kernel.org, john.fastabend@...il.com,
        davem@...emloft.net, ast@...nel.org
Subject: [net PATCH] net: tls,
 correctly account for copied bytes with multiple sk_msgs

tls_sw_do_sendpage needs to return the total number of bytes sent
regardless of how many sk_msgs are allocatedt. Unfortunately, copied
(the value we return up the stack) is zero'd before each new sk_msg
is alloced so we only return the copied size of the last sk_msg used.

The application will then believe only part of its data was sent and
send the missing chunks again. However, because the data actually was
sent the receiver will get multiple copies of the same data.

To reproduce this do multiple copies close to the max record size to
force the above scenario. Andre created a C program that can easily
generate this case so we will push a similar selftest for this to
bpf-next shortly.

The fix is to _not_ zero the copied field so that the total sent
bytes is returned.

Reported-by: Steinar H. Gunderson <steinar+kernel@...derson.no>
Reported-by: Andre Tomt <andre@...t.net>
Fixes: d829e9c4112b ("tls: convert to generic sk_msg interface")
Signed-off-by: John Fastabend <john.fastabend@...il.com>
---
 net/tls/tls_sw.c |    1 -
 1 file changed, 1 deletion(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index d93f83f77864..5fe3dfa2c5e3 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1143,7 +1143,6 @@ static int tls_sw_do_sendpage(struct sock *sk, struct page *page,

 		full_record = false;
 		record_room = TLS_MAX_PAYLOAD_SIZE - msg_pl->sg.size;
-		copied = 0;
 		copy = size;
 		if (copy >= record_room) {
 			copy = record_room;