netdev - [PATCH net] net/tls: fix sk_msg trim on fallback to copy mode

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 for Android: free password hash cracker in your pocket

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

Message-Id: <20191030160542.30295-1-jakub.kicinski@netronome.com>
Date:   Wed, 30 Oct 2019 09:05:42 -0700
From:   Jakub Kicinski <jakub.kicinski@...ronome.com>
To:     davem@...emloft.net
Cc:     netdev@...r.kernel.org, oss-drivers@...ronome.com,
        borisp@...lanox.com, aviadye@...lanox.com,
        john.fastabend@...il.com, daniel@...earbox.net,
        Jakub Kicinski <jakub.kicinski@...ronome.com>,
        syzbot+f8495bff23a879a6d0bd@...kaller.appspotmail.com,
        syzbot+6f50c99e8f6194bf363f@...kaller.appspotmail.com,
        Eric Biggers <ebiggers@...nel.org>,
        herbert@...dor.apana.org.au, glider@...gle.com,
        linux-crypto@...r.kernel.org
Subject: [PATCH net] net/tls: fix sk_msg trim on fallback to copy mode

sk_msg_trim() tries to only update curr pointer if it falls into
the trimmed region. The logic, however, does not take into the
account pointer wrapping that sk_msg_iter_var_prev() does.
This means that when the message was trimmed completely, the new
curr pointer would have the value of MAX_MSG_FRAGS - 1, which is
neither smaller than any other value, nor would it actually be
correct.

Special case the trimming to 0 length a little bit.

This bug caused the TLS code to not copy all of the message, if
zero copy filled in fewer sg entries than memcopy would need.

Big thanks to Alexander Potapenko for the non-KMSAN reproducer.

Fixes: d829e9c4112b ("tls: convert to generic sk_msg interface")
Reported-by: syzbot+f8495bff23a879a6d0bd@...kaller.appspotmail.com
Reported-by: syzbot+6f50c99e8f6194bf363f@...kaller.appspotmail.com
Signed-off-by: Jakub Kicinski <jakub.kicinski@...ronome.com>
---
Daniel, John, does this look okay?

CC: Eric Biggers <ebiggers@...nel.org>
CC: herbert@...dor.apana.org.au
CC: glider@...gle.com
CC: linux-crypto@...r.kernel.org

 net/core/skmsg.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/core/skmsg.c b/net/core/skmsg.c
index cf390e0aa73d..c42c145216b1 100644
--- a/net/core/skmsg.c
+++ b/net/core/skmsg.c
@@ -276,7 +276,10 @@ void sk_msg_trim(struct sock *sk, struct sk_msg *msg, int len)
 	 * However trimed data that has not yet been used in a copy op
 	 * does not require an update.
 	 */
-	if (msg->sg.curr >= i) {
+	if (!msg->sg.size) {
+		msg->sg.curr = 0;
+		msg->sg.copybreak = 0;
+	} else if (msg->sg.curr >= i) {
 		msg->sg.curr = i;
 		msg->sg.copybreak = msg->sg.data[i].length;
 	}
-- 
2.23.0