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: <20180806194738.981505-1-doronrk@fb.com>
Date:   Mon, 6 Aug 2018 12:47:38 -0700
From:   Doron Roberts-Kedes <doronrk@...com>
To:     "David S . Miller" <davem@...emloft.net>
CC:     Vakul Garg <vakul.garg@....com>, Dave Watson <davejwatson@...com>,
        Boris Pismenny <borisp@...lanox.com>,
        Aviad Yehezkel <aviadye@...lanox.com>,
        <netdev@...r.kernel.org>, Doron Roberts-Kedes <doronrk@...com>
Subject: [PATCH net-next,v3] net/tls: Calculate nsg for zerocopy path without skb_cow_data.

decrypt_skb fails if the number of sg elements required to map is
greater than MAX_SKB_FRAGS. As noted by Vakul Garg, nsg must always be
calculated, but skb_cow_data adds unnecessary memcpy's for the zerocopy
case.

The new function skb_nsg calculates the number of scatterlist elements
required to map the skb without the extra overhead of skb_cow_data. This
function mimics the structure of skb_to_sgvec.

Fixes: c46234ebb4d1 ("tls: RX path for ktls")
Signed-off-by: Doron Roberts-Kedes <doronrk@...com>
---
 net/tls/tls_sw.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++--
 1 file changed, 93 insertions(+), 3 deletions(-)

diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index ff3a6904a722..eb87f931a0d6 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -43,6 +43,80 @@
 
 #define MAX_IV_SIZE	TLS_CIPHER_AES_GCM_128_IV_SIZE
 
+static int __skb_nsg(struct sk_buff *skb, int offset, int len,
+		     unsigned int recursion_level)
+{
+	int start = skb_headlen(skb);
+	int i, copy = start - offset;
+	struct sk_buff *frag_iter;
+	int elt = 0;
+
+	if (unlikely(recursion_level >= 24))
+		return -EMSGSIZE;
+
+	if (copy > 0) {
+		if (copy > len)
+			copy = len;
+		elt++;
+		len -= copy;
+		if (len == 0)
+			return elt;
+		offset += copy;
+	}
+
+	for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
+		int end;
+
+		WARN_ON(start > offset + len);
+
+		end = start + skb_frag_size(&skb_shinfo(skb)->frags[i]);
+		copy = end - offset;
+		if (copy > 0) {
+			if (copy > len)
+				copy = len;
+			elt++;
+			len -= copy;
+			if (len == 0)
+				return elt;
+			offset += copy;
+		}
+		start = end;
+	}
+
+	skb_walk_frags(skb, frag_iter) {
+		int end, ret;
+
+		WARN_ON(start > offset + len);
+
+		end = start + frag_iter->len;
+		copy = end - offset;
+		if (copy > 0) {
+			if (copy > len)
+				copy = len;
+			ret = __skb_nsg(frag_iter, offset - start, copy,
+					recursion_level + 1);
+			if (unlikely(ret < 0))
+				return ret;
+			elt += ret;
+			len -= copy;
+			if (len == 0)
+				return elt;
+			offset += copy;
+		}
+		start = end;
+	}
+	BUG_ON(len);
+	return elt;
+}
+
+/* Return the number of scatterlist elements required to completely map the
+ * skb, or -EMSGSIZE if the recursion depth is exceeded.
+ */
+static int skb_nsg(struct sk_buff *skb, int offset, int len)
+{
+	return __skb_nsg(skb, offset, len, 0);
+}
+
 static int tls_do_decryption(struct sock *sk,
 			     struct scatterlist *sgin,
 			     struct scatterlist *sgout,
@@ -693,7 +767,7 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
 	struct scatterlist sgin_arr[MAX_SKB_FRAGS + 2];
 	struct scatterlist *sgin = &sgin_arr[0];
 	struct strp_msg *rxm = strp_msg(skb);
-	int ret, nsg = ARRAY_SIZE(sgin_arr);
+	int ret, nsg;
 	struct sk_buff *unused;
 
 	ret = skb_copy_bits(skb, rxm->offset + TLS_HEADER_SIZE,
@@ -704,11 +778,27 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
 
 	memcpy(iv, tls_ctx->rx.iv, TLS_CIPHER_AES_GCM_128_SALT_SIZE);
 	if (!sgout) {
-		nsg = skb_cow_data(skb, 0, &unused) + 1;
+		nsg = skb_cow_data(skb, 0, &unused);
+	} else {
+		nsg = skb_nsg(skb,
+			      rxm->offset + tls_ctx->rx.prepend_size,
+			      rxm->full_len - tls_ctx->rx.prepend_size);
+		if (nsg <= 0)
+			return nsg;
+	}
+
+	// We need one extra for ctx->rx_aad_ciphertext
+	nsg++;
+
+	if (nsg > ARRAY_SIZE(sgin_arr)) {
 		sgin = kmalloc_array(nsg, sizeof(*sgin), sk->sk_allocation);
-		sgout = sgin;
+		if (!sgin)
+			return -ENOMEM;
 	}
 
+	if (!sgout)
+		sgout = sgin;
+
 	sg_init_table(sgin, nsg);
 	sg_set_buf(&sgin[0], ctx->rx_aad_ciphertext, TLS_AAD_SPACE_SIZE);
 
-- 
2.17.1

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ