TLS 1.2 and TLS 1.3 both permit zero-length application_data
records as a traffic-analysis countermeasure (RFC 5246 Section
6.2.1 and RFC 8446 Section 5.1). A record that decrypts to
full_len == 0 advances neither len nor decrypted, so
tls_sw_recvmsg() keeps reading empty records for as long as they
arrive. The peek arm and the async arm queue each empty record on
rx_list, and a peer that streams empty records grows rx_list
without bound. The growth has not been reproduced.
Consume an empty data record as soon as the receive loop has it,
before the paths diverge on darg.zc. Freeing the skb requires its
decryption to have completed, and an empty record is already
decrypted synchronously. The new branch sets MSG_EOR itself,
because the record no longer reaches the assignment at the bottom
of the loop. The peek arm does not set the flag, so after the
change a MSG_PEEK receive that reaches an empty record reports
MSG_EOR where it did not before. Test for a pending signal in the
branch as well, and drop the flag that deferred the test to the
next iteration.
After this change, tls_sw_recvmsg() still reads empty records for
as long as they arrive, until a signal is pending.
Fixes: 692d7b5d1f91 ("tls: Fix recvmsg() to be able to peek across multiple
records")
Reported-by: sashiko-bot <[email protected]>
Closes:
https://sashiko.dev/#/patchset/[email protected]?part=1
Signed-off-by: Chuck Lever <[email protected]>
---
net/tls/tls_sw.c | 35 +++++++++++++++++++++++------------
1 file changed, 23 insertions(+), 12 deletions(-)
diff --git a/net/tls/tls_sw.c b/net/tls/tls_sw.c
index fe9c5c21dadc..550308410ff1 100644
--- a/net/tls/tls_sw.c
+++ b/net/tls/tls_sw.c
@@ -1832,7 +1832,6 @@ int tls_sw_recvmsg(struct sock *sk,
bool is_peek = flags & MSG_PEEK;
bool rx_more = false;
bool released = true;
- bool nodata = false;
bool zc_capable;
if (unlikely(flags & MSG_ERRQUEUE))
@@ -1867,14 +1866,6 @@ int tls_sw_recvmsg(struct sock *sk,
struct tls_decrypt_arg darg;
int to_decrypt, chunk;
- /* A run of empty records advances neither loop bound, and
- * tls_rx_rec_wait() tests for a signal only after sleeping.
- */
- if (nodata && signal_pending(current)) {
- err = tls_rx_signal_err(sk, flags & MSG_DONTWAIT);
- goto recv_end;
- }
-
err = tls_rx_rec_wait(sk, flags & MSG_DONTWAIT,
released, !!(decrypted + copied));
if (err <= 0)
@@ -1892,8 +1883,8 @@ int tls_sw_recvmsg(struct sock *sk,
darg.zc = true;
/* Do not use async mode if record is non-data or empty.
- * Once a record has gone async, recv_end discards the
- * error that ends a run of empty records.
+ * The receive loop frees an empty record's skb, which an
+ * async decrypt would still be using.
*/
if (tlm->control == TLS_RECORD_TYPE_DATA)
darg.async = ctx->async_capable && to_decrypt;
@@ -1930,9 +1921,29 @@ int tls_sw_recvmsg(struct sock *sk,
/* TLS 1.3 may have updated the length by more than overhead */
rxm = strp_msg(darg.skb);
chunk = rxm->full_len;
- nodata = !chunk;
tls_rx_rec_done(ctx);
+ /* Keep an empty record off rx_list. On the zero-copy path
+ * darg.skb is the strparser's, released by tls_rx_rec_done().
+ */
+ if (!chunk && control == TLS_RECORD_TYPE_DATA) {
+ if (!darg.zc)
+ consume_skb(darg.skb);
+
+ /* An empty record still marks a boundary. */
+ msg->msg_flags |= MSG_EOR;
+
+ /* A run of empty records advances neither loop bound,
+ * and tls_rx_rec_wait() tests for a signal only after
+ * sleeping.
+ */
+ if (signal_pending(current)) {
+ err = tls_rx_signal_err(sk, flags &
MSG_DONTWAIT);
+ goto recv_end;
+ }
+ continue;
+ }
+
if (!darg.zc) {
bool partially_consumed = chunk > len;
struct sk_buff *skb = darg.skb;
--
2.55.0
