From: Jakub Kicinski <kuba@kernel.org>
To: davem@davemloft.net, pabeni@redhat.com
Cc: netdev@vger.kernel.org, borisp@nvidia.com,
john.fastabend@gmail.com, daniel@iogearbox.net,
vfedorenko@novek.ru, Jakub Kicinski <kuba@kernel.org>
Subject: [PATCH net-next 02/11] tls: rx: don't report text length from the bowels of decrypt
Date: Fri, 8 Apr 2022 11:31:25 -0700 [thread overview]
Message-ID: <20220408183134.1054551-3-kuba@kernel.org> (raw)
In-Reply-To: <20220408183134.1054551-1-kuba@kernel.org>
We plumb pointer to chunk all the way to the decryption method.
It's set to the length of the text when decrypt_skb_update()
returns.
I think the code is written this way because original TLS
implementation passed &chunk to zerocopy_from_iter() and this
was carried forward as the code gotten more complex, without
any refactoring.
The fix for peek() introduced a new variable - to_decrypt
which for all practical purposes is what chunk is going to
get set to. Spare ourselves the pointer passing, use to_decrypt.
Use this opportunity to clean things up a little further.
Note that chunk / to_decrypt was mostly needed for the async
path, since the sync path would access rxm->full_len (decryption
transforms full_len from record size to text size). Use the
right source of truth more explicitly.
We have three cases:
- async - it's TLS 1.2 only, so chunk == to_decrypt, but we
need the min() because to_decrypt is a whole record
and we don't want to underflow len. Note that we can't
handle partial record by falling back to sync as it
would introduce reordering against records in flight.
- zc - again, TLS 1.2 only for now, so chunk == to_decrypt,
we don't do zc if len < to_decrypt, no need to check again.
- normal - it already handles chunk > len, we can factor out the
assignment to rxm->full_len and share it with zc.
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
---
net/tls/tls_sw.c | 33 ++++++++++++++-------------------
1 file changed, 14 insertions(+), 19 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index 86f77f8b825e..c321c5f85fbe 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1412,7 +1412,7 @@ static int tls_setup_from_iter(struct iov_iter *from,
static int decrypt_internal(struct sock *sk, struct sk_buff *skb,
struct iov_iter *out_iov,
struct scatterlist *out_sg,
- int *chunk, bool *zc, bool async)
+ bool *zc, bool async)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_sw_context_rx *ctx = tls_sw_ctx_rx(tls_ctx);
@@ -1526,7 +1526,6 @@ static int decrypt_internal(struct sock *sk, struct sk_buff *skb,
(n_sgout - 1));
if (err < 0)
goto fallback_to_reg_recv;
- *chunk = data_len;
} else if (out_sg) {
memcpy(sgout, out_sg, n_sgout * sizeof(*sgout));
} else {
@@ -1536,7 +1535,6 @@ static int decrypt_internal(struct sock *sk, struct sk_buff *skb,
fallback_to_reg_recv:
sgout = sgin;
pages = 0;
- *chunk = data_len;
*zc = false;
}
@@ -1555,8 +1553,7 @@ static int decrypt_internal(struct sock *sk, struct sk_buff *skb,
}
static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
- struct iov_iter *dest, int *chunk, bool *zc,
- bool async)
+ struct iov_iter *dest, bool *zc, bool async)
{
struct tls_context *tls_ctx = tls_get_ctx(sk);
struct tls_prot_info *prot = &tls_ctx->prot_info;
@@ -1580,7 +1577,7 @@ static int decrypt_skb_update(struct sock *sk, struct sk_buff *skb,
}
}
- err = decrypt_internal(sk, skb, dest, NULL, chunk, zc, async);
+ err = decrypt_internal(sk, skb, dest, NULL, zc, async);
if (err < 0) {
if (err == -EINPROGRESS)
tls_advance_record_sn(sk, prot, &tls_ctx->rx);
@@ -1607,9 +1604,8 @@ int decrypt_skb(struct sock *sk, struct sk_buff *skb,
struct scatterlist *sgout)
{
bool zc = true;
- int chunk;
- return decrypt_internal(sk, skb, NULL, sgout, &chunk, &zc, false);
+ return decrypt_internal(sk, skb, NULL, sgout, &zc, false);
}
static bool tls_sw_advance_skb(struct sock *sk, struct sk_buff *skb,
@@ -1799,9 +1795,8 @@ int tls_sw_recvmsg(struct sock *sk,
num_async = 0;
while (len && (decrypted + copied < target || ctx->recv_pkt)) {
bool retain_skb = false;
+ int to_decrypt, chunk;
bool zc = false;
- int to_decrypt;
- int chunk = 0;
bool async_capable;
bool async = false;
@@ -1838,7 +1833,7 @@ int tls_sw_recvmsg(struct sock *sk,
async_capable = false;
err = decrypt_skb_update(sk, skb, &msg->msg_iter,
- &chunk, &zc, async_capable);
+ &zc, async_capable);
if (err < 0 && err != -EINPROGRESS) {
tls_err_abort(sk, -EBADMSG);
goto recv_end;
@@ -1876,8 +1871,13 @@ int tls_sw_recvmsg(struct sock *sk,
}
}
- if (async)
+ if (async) {
+ /* TLS 1.2-only, to_decrypt must be text length */
+ chunk = min_t(int, to_decrypt, len);
goto pick_next_record;
+ }
+ /* TLS 1.3 may have updated the length by more than overhead */
+ chunk = rxm->full_len;
if (!zc) {
if (bpf_strp_enabled) {
@@ -1893,11 +1893,9 @@ int tls_sw_recvmsg(struct sock *sk,
}
}
- if (rxm->full_len > len) {
+ if (chunk > len) {
retain_skb = true;
chunk = len;
- } else {
- chunk = rxm->full_len;
}
err = skb_copy_datagram_msg(skb, rxm->offset,
@@ -1912,9 +1910,6 @@ int tls_sw_recvmsg(struct sock *sk,
}
pick_next_record:
- if (chunk > len)
- chunk = len;
-
decrypted += chunk;
len -= chunk;
@@ -2016,7 +2011,7 @@ ssize_t tls_sw_splice_read(struct socket *sock, loff_t *ppos,
if (!skb)
goto splice_read_end;
- err = decrypt_skb_update(sk, skb, NULL, &chunk, &zc, false);
+ err = decrypt_skb_update(sk, skb, NULL, &zc, false);
if (err < 0) {
tls_err_abort(sk, -EBADMSG);
goto splice_read_end;
--
2.34.1
next prev parent reply other threads:[~2022-04-08 18:31 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-04-08 18:31 [PATCH net-next 00/11] tls: rx: random refactoring part 2 Jakub Kicinski
2022-04-08 18:31 ` [PATCH net-next 01/11] tls: rx: drop unnecessary arguments from tls_setup_from_iter() Jakub Kicinski
2022-04-08 18:31 ` Jakub Kicinski [this message]
2022-04-08 18:31 ` [PATCH net-next 03/11] tls: rx: wrap decryption arguments in a structure Jakub Kicinski
2022-04-08 18:31 ` [PATCH net-next 04/11] tls: rx: simplify async wait Jakub Kicinski
2022-04-08 18:31 ` [PATCH net-next 05/11] tls: rx: factor out writing ContentType to cmsg Jakub Kicinski
2022-04-08 18:31 ` [PATCH net-next 06/11] tls: rx: don't handle async in tls_sw_advance_skb() Jakub Kicinski
2022-04-08 18:31 ` [PATCH net-next 07/11] tls: rx: don't track the async count Jakub Kicinski
2022-04-08 18:31 ` [PATCH net-next 08/11] tls: rx: pull most of zc check out of the loop Jakub Kicinski
2022-04-08 18:31 ` [PATCH net-next 09/11] tls: rx: inline consuming the skb at the end " Jakub Kicinski
2022-04-08 18:31 ` [PATCH net-next 10/11] tls: rx: clear ctx->recv_pkt earlier Jakub Kicinski
2022-05-18 12:43 ` Artem Savkov
2022-04-08 18:31 ` [PATCH net-next 11/11] tls: rx: jump out for cases which need to leave skb on list Jakub Kicinski
2022-04-08 19:03 ` [PATCH net-next 00/11] tls: rx: random refactoring part 2 John Fastabend
2022-04-10 16:40 ` patchwork-bot+netdevbpf
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220408183134.1054551-3-kuba@kernel.org \
--to=kuba@kernel.org \
--cc=borisp@nvidia.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=john.fastabend@gmail.com \
--cc=netdev@vger.kernel.org \
--cc=pabeni@redhat.com \
--cc=vfedorenko@novek.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).