A peer may send a zero-length application_data record. After
decryption such a record has full_len == 0, so tls_sw_splice_read()
splices zero bytes and returns zero. A zero return from a splice
read signals EOF, and the caller tears down a connection that is
still live.
Consume such a record and fetch the next one, as tls_sw_recvmsg()
already does. A peer that streams empty records can hold the
splicing task in that loop, so test for a pending signal where the
record is consumed.
Where an empty record precedes a close_notify, splice(2) now returns
-EINVAL from the control-record test rather than the zero it
returned before. That zero was the false EOF this patch removes, and
a splice that reaches an alert record has always reported -EINVAL.
The open-coded test in tls_sw_read_sock() becomes
tls_rx_empty_data_rec(); the tls_sw_recvmsg() paths reuse it and
tls_rx_intr_errno() in the patches that follow.
Fixes: c46234ebb4d1 ("tls: RX path for ktls")
Signed-off-by: Chuck Lever <[email protected]>
---
net/tls/tls_sw.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++-------
1 file changed, 46 insertions(+), 7 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index d45c945a3d1d..44ae0fad780d 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1788,6 +1788,24 @@ static void tls_rx_reader_unlock(struct sock *sk, struct
tls_sw_context_rx *ctx)
release_sock(sk);
}
+/* TLS 1.2 and TLS 1.3 both permit a zero-length application_data
+ * record as a traffic-analysis countermeasure (RFC 5246, Section
+ * 6.2.1; RFC 8446, Section 5.1).
+ */
+static bool tls_rx_empty_data_rec(int len, unsigned char control)
+{
+ return !len && control == TLS_RECORD_TYPE_DATA;
+}
+
+/* sock_intr_errno() maps the zero timeo of a reader that cannot wait
+ * to -EINTR, but such a reader has no blocking to interrupt. The rest
+ * of the receive side reports that case as -EAGAIN.
+ */
+static int tls_rx_intr_errno(long timeo)
+{
+ return timeo ? sock_intr_errno(timeo) : -EAGAIN;
+}
+
int tls_sw_recvmsg(struct sock *sk,
struct msghdr *msg,
size_t len,
@@ -1991,6 +2009,7 @@ ssize_t tls_sw_splice_read(struct socket *sock, loff_t
*ppos,
struct sock *sk = sock->sk;
struct tls_msg *tlm;
struct sk_buff *skb;
+ bool released = true;
ssize_t copied = 0;
int chunk;
int err;
@@ -1999,13 +2018,14 @@ ssize_t tls_sw_splice_read(struct socket *sock, loff_t
*ppos,
if (err < 0)
return err;
+retry:
if (!skb_queue_empty(&ctx->rx_list)) {
skb = __skb_dequeue(&ctx->rx_list);
} else {
struct tls_decrypt_arg darg;
err = tls_rx_rec_wait(sk, flags & SPLICE_F_NONBLOCK,
- true, false);
+ released, false);
if (err <= 0)
goto splice_read_end;
@@ -2017,6 +2037,11 @@ ssize_t tls_sw_splice_read(struct socket *sock, loff_t
*ppos,
tls_rx_rec_done(ctx);
skb = darg.skb;
+
+ /* The socket lock stays held to the retry, so the
+ * anchor this wait loaded survives it.
+ */
+ released = false;
}
rxm = strp_msg(skb);
@@ -2028,6 +2053,21 @@ ssize_t tls_sw_splice_read(struct socket *sock, loff_t
*ppos,
goto splice_requeue;
}
+ /* Splicing an empty data record delivers zero bytes, which the
+ * caller reads as EOF. tls_rx_rec_wait() skips its signal check
+ * while a record is parsed, so test for a signal here.
+ */
+ if (tls_rx_empty_data_rec(rxm->full_len, tlm->control)) {
+ long timeo = sock_rcvtimeo(sk, flags & SPLICE_F_NONBLOCK);
+
+ consume_skb(skb);
+ if (signal_pending(current)) {
+ err = tls_rx_intr_errno(timeo);
+ goto splice_read_end;
+ }
+ goto retry;
+ }
+
chunk = min_t(unsigned int, rxm->full_len, len);
copied = skb_splice_bits(skb, sk, rxm->offset, pipe, chunk, flags);
if (copied < 0)
@@ -2122,13 +2162,12 @@ int tls_sw_read_sock(struct sock *sk, read_descriptor_t
*desc,
goto read_sock_requeue;
}
- /* An empty data record (legal in TLS 1.3) gives a zero
- * read_actor return, indistinguishable from the consumer
- * stalling; the used <= 0 path would requeue it at the
- * head of rx_list and block all later records. Consume it
- * here instead.
+ /* An empty data record gives a zero read_actor return,
+ * indistinguishable from the consumer stalling; the
+ * used <= 0 path would requeue it at the head of rx_list
+ * and block all later records. Consume it here instead.
*/
- if (rxm->full_len == 0) {
+ if (tls_rx_empty_data_rec(rxm->full_len, tlm->control)) {
err = 0;
consume_skb(skb);
if (!nodata_deadline) {
--
2.54.0
