ktls: Fix invalid memory access on retry with moving write buffer

kTLS write is using application buffer always without a memory copy.
And it completely ignores SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER as a
result.  If the user frees or re-uses the original buffer and retries
the send on SSL_ERROR_WANT_WRITE, the code will read and send the data
from the original already freed buffer sending whatever happens to be
in that memory now and corrupting the message, potentially crashing
the application as well.

Fix by making a copy if we can't send the whole thing right away and
the moving write buffer is configured.

This preserves the zero-copy semantics for the happy path and avoids
the invalid memory access and data corruption when retry is necessary.
The copy is done in the common code as it is hard to preserve the
zero-copy behavior otherwise.

Test is added that reproduces the issue.  It may be possible to modify
the existing kTLS test to conditionally enable the modes and do the
BIO swap, but it feels like the issue deserves a separate one.

The test doesn't rely on any specific cypher or TLS version, so only
one combination is checked, but it should be enough.

There is no TLS_BUFFER_set_len() and the original kTLS code never
sets it, so not setting it on the copy either for now.

Fixes: 50ec750567e0 "ssl: Linux TLS Tx Offload"
Fixes #21202

Assisted-by: claude-opus-4.6
Reviewed-by: Matt Caswell <matt@openssl.foundation>
Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
MergeDate: Mon Jun  1 07:29:13 2026
(Merged from https://github.com/openssl/openssl/pull/31146)

diff --git a/ssl/record/methods/ktls_meth.c b/ssl/record/methods/ktls_meth.c
index bfdcc24687f25348eaff82efac1cf02585bda8ce..a6fc77ea68ff30a3f66afa0dc17f416ba8b913c2 100644 (file)
--- a/ssl/record/methods/ktls_meth.c
+++ b/ssl/record/methods/ktls_meth.c
@@ -472,11 +472,16 @@ static int ktls_initialise_write_packets(OSSL_RECORD_LAYER *rl,
     wb = &bufs[0];
     wb->type = templates[0].type;
 
+    /*
+     * Free any internal buffer allocated during a previous write retry
+     * (see tls_retry_write_records).  App buffers are not ours to free.
+     */
+    if (!TLS_BUFFER_is_app_buffer(wb))
+        OPENSSL_free(TLS_BUFFER_get_buf(wb));
+
     /*
      * ktls doesn't modify the buffer, but to avoid a warning we need
      * to discard the const qualifier.
-     * This doesn't leak memory because the buffers have never been allocated
-     * with KTLS
      */
     TLS_BUFFER_set_buf(wb, (unsigned char *)templates[0].buf);
     TLS_BUFFER_set_offset(wb, 0);
@@ -547,15 +552,6 @@ static int ktls_alloc_buffers(OSSL_RECORD_LAYER *rl)
     return tls_alloc_buffers(rl);
 }
 
-static int ktls_free_buffers(OSSL_RECORD_LAYER *rl)
-{
-    /* We use the application buffer directly for writing */
-    if (rl->direction == OSSL_RECORD_DIRECTION_WRITE)
-        return 1;
-
-    return tls_free_buffers(rl);
-}
-
 static struct record_functions_st ossl_ktls_funcs = {
     ktls_set_crypto_state,
     ktls_cipher,
@@ -602,5 +598,5 @@ const OSSL_RECORD_METHOD ossl_ktls_record_method = {
     NULL,
     tls_increment_sequence_ctr,
     ktls_alloc_buffers,
-    ktls_free_buffers
+    tls_free_buffers
 };

diff --git a/ssl/record/methods/tls_common.c b/ssl/record/methods/tls_common.c
index c8e5218226df6bf42882d470b94859a8a175a09d..c8f7b6dd46bd59dc848fc85ff7eb66fe1b130847 100644 (file)
--- a/ssl/record/methods/tls_common.c
+++ b/ssl/record/methods/tls_common.c
@@ -1918,6 +1918,28 @@ int tls_retry_write_records(OSSL_RECORD_LAYER *rl)
                 tls_release_write_buffer(rl);
             return OSSL_RECORD_RETURN_SUCCESS;
         } else if (i <= 0) {
+            /*
+             * If the app buffer is used directly (kTLS) and the caller is
+             * allowed to move it, copy the unsent data so the original
+             * buffer can be safely released.
+             */
+            if (TLS_BUFFER_is_app_buffer(thiswb)
+                && (rl->mode & SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER) != 0) {
+                size_t left = TLS_BUFFER_get_left(thiswb);
+                unsigned char *buf;
+
+                buf = OPENSSL_malloc(left);
+                if (buf == NULL) {
+                    RLAYERfatal(rl, SSL_AD_INTERNAL_ERROR, ERR_R_INTERNAL_ERROR);
+                    return OSSL_RECORD_RETURN_FATAL;
+                }
+                memcpy(buf,
+                    TLS_BUFFER_get_buf(thiswb) + TLS_BUFFER_get_offset(thiswb),
+                    left);
+                TLS_BUFFER_set_buf(thiswb, buf);
+                TLS_BUFFER_set_offset(thiswb, 0);
+                TLS_BUFFER_set_app_buffer(thiswb, 0);
+            }
             if (rl->isdtls) {
                 /*
                  * For DTLS, just drop it. That's kind of the whole point in

diff --git a/test/sslapitest.c b/test/sslapitest.c
index 2cf22bd9b8949e0c77a1dcbc785ca01e6a462d84..e8c5da352f1d55f98b9a6b0d487ca2eab2e9bfdf 100644 (file)
--- a/test/sslapitest.c
+++ b/test/sslapitest.c
@@ -1461,6 +1461,144 @@ end:
     return testresult;
 }
 
+#ifndef OSSL_NO_USABLE_TLS1_3
+/*
+ * Test kTLS with SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER: retry SSL_write() after
+ * SSL_ERROR_WANT_WRITE using a different buffer pointer (same content) and
+ * verify that the data arrives intact.
+ */
+static int test_ktls_moving_write_buffer(void)
+{
+    SSL_CTX *cctx = NULL, *sctx = NULL;
+    SSL *clientssl = NULL, *serverssl = NULL;
+    BIO *bio_retry = NULL, *bio_orig = NULL;
+    int testresult = 0, cfd = -1, sfd = -1;
+    unsigned char *buf_orig = NULL, *buf_retry = NULL;
+    unsigned char outbuf[1024];
+    const size_t bufsz = sizeof(outbuf);
+    size_t written, readbytes, totread = 0, i;
+
+    /* kTLS requires real sockets */
+    if (!TEST_true(create_test_sockets(&cfd, &sfd, SOCK_STREAM, NULL)))
+        goto end;
+
+    /* Skip if the kernel does not support kTLS */
+    if (!ktls_chk_platform(cfd)) {
+        testresult = TEST_skip("Kernel does not support KTLS");
+        goto end;
+    }
+
+    if (!TEST_true(create_ssl_ctx_pair(libctx,
+            TLS_server_method(), TLS_client_method(),
+            TLS1_3_VERSION, TLS1_3_VERSION,
+            &sctx, &cctx, cert, privkey)))
+        goto end;
+
+    if (!TEST_true(SSL_CTX_set_ciphersuites(cctx, "TLS_AES_128_GCM_SHA256"))
+        || !TEST_true(SSL_CTX_set_ciphersuites(sctx, "TLS_AES_128_GCM_SHA256")))
+        goto end;
+
+    if (!TEST_true(create_ssl_objects2(sctx, cctx, &serverssl,
+            &clientssl, sfd, cfd)))
+        goto end;
+
+    /* Enable kTLS on the writing side (client) */
+    if (!TEST_true(SSL_set_options(clientssl, SSL_OP_ENABLE_KTLS)))
+        goto end;
+
+    SSL_set_mode(clientssl, SSL_MODE_ACCEPT_MOVING_WRITE_BUFFER);
+    SSL_set_mode(clientssl, SSL_MODE_ENABLE_PARTIAL_WRITE);
+
+    if (!TEST_true(create_ssl_connection(serverssl, clientssl, SSL_ERROR_NONE)))
+        goto end;
+
+    /* Get a reference to the original BIO to replace it later. */
+    bio_orig = SSL_get_wbio(clientssl);
+    if (!TEST_ptr(bio_orig) || !TEST_true(BIO_up_ref(bio_orig))) {
+        bio_orig = NULL;
+        goto end;
+    }
+
+    /* Skip if kTLS TX was not activated for this cipher */
+    if (!BIO_get_ktls_send(bio_orig)) {
+        testresult = TEST_skip("kTLS send not supported");
+        goto end;
+    }
+
+    /* Swap write BIO to force WANT_WRITE */
+    bio_retry = BIO_new(bio_s_always_retry());
+    if (!TEST_ptr(bio_retry))
+        goto end;
+
+    SSL_set0_wbio(clientssl, bio_retry);
+    bio_retry = NULL; /* ownership transferred to clientssl */
+
+    /* Allocate two buffers with identical content but different addresses */
+    buf_orig = OPENSSL_malloc(bufsz);
+    buf_retry = OPENSSL_malloc(bufsz);
+    if (!TEST_ptr(buf_orig) || !TEST_ptr(buf_retry))
+        goto end;
+
+    for (i = 0; i < bufsz; i++)
+        buf_orig[i] = buf_retry[i] = (unsigned char)(i & 0xff);
+
+    /* First write attempt - will fail with WANT_WRITE */
+    if (!TEST_false(SSL_write_ex(clientssl, buf_orig, bufsz, &written))
+        || !TEST_int_eq(SSL_get_error(clientssl, 0), SSL_ERROR_WANT_WRITE))
+        goto end;
+
+    /* Restore the real socket BIO so the retry can actually send data */
+    SSL_set0_wbio(clientssl, bio_orig);
+    bio_orig = NULL;
+
+    /* Poison and free the original buffer */
+    memset(buf_orig, 0xDE, bufsz);
+    OPENSSL_free(buf_orig);
+    buf_orig = NULL;
+
+    /* Retry with a different buffer pointer */
+    if (!TEST_true(SSL_write_ex(clientssl, buf_retry, bufsz, &written)))
+        goto end;
+
+    /* Read the data on the server side */
+    totread = 0;
+    while (totread < bufsz) {
+        if (!SSL_read_ex(serverssl, outbuf + totread, bufsz - totread,
+                &readbytes)) {
+            if (!TEST_int_eq(SSL_get_error(serverssl, 0), SSL_ERROR_WANT_READ))
+                goto end;
+        } else {
+            totread += readbytes;
+        }
+    }
+
+    /* Verify data integrity */
+    if (!TEST_mem_eq(buf_retry, bufsz, outbuf, totread))
+        goto end;
+
+    testresult = 1;
+end:
+    OPENSSL_free(buf_orig);
+    OPENSSL_free(buf_retry);
+    if (clientssl != NULL) {
+        SSL_shutdown(clientssl);
+        SSL_free(clientssl);
+    }
+    if (serverssl != NULL) {
+        SSL_shutdown(serverssl);
+        SSL_free(serverssl);
+    }
+    SSL_CTX_free(sctx);
+    SSL_CTX_free(cctx);
+    BIO_free_all(bio_orig);
+    if (cfd != -1)
+        close(cfd);
+    if (sfd != -1)
+        close(sfd);
+    return testresult;
+}
+#endif /* !defined(OSSL_NO_USABLE_TLS1_3) */
+
 static struct ktls_test_cipher {
     int tls_version;
     const char *cipher;
@@ -14931,6 +15069,9 @@ int setup_tests(void)
     ADD_ALL_TESTS(test_ktls, NUM_KTLS_TEST_CIPHERS * 4);
     ADD_ALL_TESTS(test_ktls_sendfile, NUM_KTLS_TEST_CIPHERS * 2);
 #endif
+#ifndef OSSL_NO_USABLE_TLS1_3
+    ADD_TEST(test_ktls_moving_write_buffer);
+#endif
 #endif
     ADD_TEST(test_large_message_tls);
     ADD_TEST(test_large_message_tls_read_ahead);