[PATCH v2] backends/cryptodev-lkcf: fix use-after-free in session lifecycle

Gonglei posted 1 patch 1 month, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260416061933.1982-1-arei.gonglei@huawei.com
Maintainers: "Gonglei (Arei)" <arei.gonglei@huawei.com>, zhenwei pi <zhenwei.pi@linux.dev>
backends/cryptodev-lkcf.c | 59 ++++++++++++++++++++++++++++++++++++++-
1 file changed, 58 insertions(+), 1 deletion(-)
[PATCH v2] backends/cryptodev-lkcf: fix use-after-free in session lifecycle
Posted by Gonglei 1 month, 2 weeks ago
The cryptodev-lkcf backend had a race condition where session close
could free a session while tasks using that session were still pending
in the queue. This leads to use-after-free when the worker thread
later accesses the freed session pointer.

Add reference counting (in_use) and pending_close flag to ensure:
- New operations are rejected when a session is closing
- Session close waits for all in-flight tasks to complete
- No use-after-free can occur

Fixes: CVE-2026-6288
Fixes: 39fff6f3e8 ("cryptodev: Add a lkcf-backend for cryptodev")
Reported-by: Buzzy <buzzy0257@gmail.com>
Signed-off-by: Gonglei <arei.gonglei@huawei.com>
Tested-by: Buzzy <buzzy0257@gmail.com>
---
Changes:

v2: 
 * moved sess->pending_close checking before @task allocated
   in cryptodev_lkcf_operation().

---
 backends/cryptodev-lkcf.c | 59 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 58 insertions(+), 1 deletion(-)

diff --git a/backends/cryptodev-lkcf.c b/backends/cryptodev-lkcf.c
index 40c7bd3c5a..3a93c81372 100644
--- a/backends/cryptodev-lkcf.c
+++ b/backends/cryptodev-lkcf.c
@@ -66,6 +66,9 @@ typedef struct CryptoDevBackendLKCFSession {
     size_t keylen;
     QCryptoAkCipherKeyType keytype;
     QCryptoAkCipherOptions akcipher_opts;
+    int in_use;  /* number of tasks currently using this session */
+    /* session close requested, waiting for in_use to become 0 */
+    bool pending_close;
 } CryptoDevBackendLKCFSession;
 
 typedef struct CryptoDevLKCFTask CryptoDevLKCFTask;
@@ -428,6 +431,18 @@ out:
     if (key_id >= 0) {
         keyctl_unlink(key_id, KCTL_KEY_RING);
     }
+
+    /*
+     * Decrement session in_use counter and signal if session is pending close.
+     * This allows close_session to proceed after all tasks complete.
+     */
+    qemu_mutex_lock(&task->lkcf->mutex);
+    task->sess->in_use--;
+    if (task->sess->pending_close && task->sess->in_use == 0) {
+        qemu_cond_broadcast(&task->lkcf->cond);
+    }
+    qemu_mutex_unlock(&task->lkcf->mutex);
+
     task->status = status;
 
     qemu_mutex_lock(&task->lkcf->rsp_mutex);
@@ -486,12 +501,32 @@ static int cryptodev_lkcf_operation(
         return -VIRTIO_CRYPTO_INVSESS;
     }
 
-    sess = lkcf->sess[op_info->session_id];
     if (algtype != QCRYPTODEV_BACKEND_ALGO_TYPE_ASYM) {
         error_report("algtype not supported: %u", algtype);
         return -VIRTIO_CRYPTO_NOTSUPP;
     }
 
+    /*
+     * Check if session is pending close and increment in_use counter
+     * atomically under the mutex. This prevents the session from being
+     * freed while a task is pending.
+     */
+    qemu_mutex_lock(&lkcf->mutex);
+    sess = lkcf->sess[op_info->session_id];
+    if (!sess) {
+        qemu_mutex_unlock(&lkcf->mutex);
+        error_report("Cannot find a valid session id: %" PRIu64 "",
+                     op_info->session_id);
+        return -VIRTIO_CRYPTO_INVSESS;
+    }
+    if (sess->pending_close) {
+        qemu_mutex_unlock(&lkcf->mutex);
+        error_report("Session %" PRIu64 " is closing", op_info->session_id);
+        return -VIRTIO_CRYPTO_INVSESS;
+    }
+    sess->in_use++;
+    qemu_mutex_unlock(&lkcf->mutex);
+
     task = g_new0(CryptoDevLKCFTask, 1);
     task->op_info = op_info;
     task->cb = op_info->cb;
@@ -606,8 +641,30 @@ static int cryptodev_lkcf_close_session(CryptoDevBackend *backend,
     CryptoDevBackendLKCFSession *session;
 
     assert(session_id < MAX_SESSIONS && lkcf->sess[session_id]);
+
+    qemu_mutex_lock(&lkcf->mutex);
     session = lkcf->sess[session_id];
+
+    /*
+     * Mark session as pending close. New operations using this session
+     * will be rejected. We hold the mutex until in_use becomes 0 to
+     * prevent race conditions.
+     */
+    session->pending_close = true;
+
+    /*
+     * Wait for all in-flight tasks using this session to complete.
+     * The worker thread decrements in_use after task execution.
+     */
+    while (session->in_use > 0) {
+        qemu_cond_wait(&lkcf->cond, &lkcf->mutex);
+    }
+
+    /*
+     * Now safe to remove session and free resources.
+     */
     lkcf->sess[session_id] = NULL;
+    qemu_mutex_unlock(&lkcf->mutex);
 
     g_free(session->key);
     g_free(session);
-- 
2.43.0
Re: [PATCH v2] backends/cryptodev-lkcf: fix use-after-free in session lifecycle
Posted by Daniel P. Berrangé 2 weeks, 5 days ago
On Thu, Apr 16, 2026 at 02:19:32PM +0800, Gonglei wrote:
> The cryptodev-lkcf backend had a race condition where session close
> could free a session while tasks using that session were still pending
> in the queue. This leads to use-after-free when the worker thread
> later accesses the freed session pointer.
> 
> Add reference counting (in_use) and pending_close flag to ensure:
> - New operations are rejected when a session is closing
> - Session close waits for all in-flight tasks to complete
> - No use-after-free can occur
> 
> Fixes: CVE-2026-6288
> Fixes: 39fff6f3e8 ("cryptodev: Add a lkcf-backend for cryptodev")
> Reported-by: Buzzy <buzzy0257@gmail.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Tested-by: Buzzy <buzzy0257@gmail.com>
> ---

This patch was reviewed quite a while ago, but I don't see it
merged in git yet, so just wanted to double-check it was
queued for a future pull request and not lost. 

> Changes:
> 
> v2: 
>  * moved sess->pending_close checking before @task allocated
>    in cryptodev_lkcf_operation().
> 
> ---
>  backends/cryptodev-lkcf.c | 59 ++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/backends/cryptodev-lkcf.c b/backends/cryptodev-lkcf.c
> index 40c7bd3c5a..3a93c81372 100644
> --- a/backends/cryptodev-lkcf.c
> +++ b/backends/cryptodev-lkcf.c
> @@ -66,6 +66,9 @@ typedef struct CryptoDevBackendLKCFSession {
>      size_t keylen;
>      QCryptoAkCipherKeyType keytype;
>      QCryptoAkCipherOptions akcipher_opts;
> +    int in_use;  /* number of tasks currently using this session */
> +    /* session close requested, waiting for in_use to become 0 */
> +    bool pending_close;
>  } CryptoDevBackendLKCFSession;
>  
>  typedef struct CryptoDevLKCFTask CryptoDevLKCFTask;
> @@ -428,6 +431,18 @@ out:
>      if (key_id >= 0) {
>          keyctl_unlink(key_id, KCTL_KEY_RING);
>      }
> +
> +    /*
> +     * Decrement session in_use counter and signal if session is pending close.
> +     * This allows close_session to proceed after all tasks complete.
> +     */
> +    qemu_mutex_lock(&task->lkcf->mutex);
> +    task->sess->in_use--;
> +    if (task->sess->pending_close && task->sess->in_use == 0) {
> +        qemu_cond_broadcast(&task->lkcf->cond);
> +    }
> +    qemu_mutex_unlock(&task->lkcf->mutex);
> +
>      task->status = status;
>  
>      qemu_mutex_lock(&task->lkcf->rsp_mutex);
> @@ -486,12 +501,32 @@ static int cryptodev_lkcf_operation(
>          return -VIRTIO_CRYPTO_INVSESS;
>      }
>  
> -    sess = lkcf->sess[op_info->session_id];
>      if (algtype != QCRYPTODEV_BACKEND_ALGO_TYPE_ASYM) {
>          error_report("algtype not supported: %u", algtype);
>          return -VIRTIO_CRYPTO_NOTSUPP;
>      }
>  
> +    /*
> +     * Check if session is pending close and increment in_use counter
> +     * atomically under the mutex. This prevents the session from being
> +     * freed while a task is pending.
> +     */
> +    qemu_mutex_lock(&lkcf->mutex);
> +    sess = lkcf->sess[op_info->session_id];
> +    if (!sess) {
> +        qemu_mutex_unlock(&lkcf->mutex);
> +        error_report("Cannot find a valid session id: %" PRIu64 "",
> +                     op_info->session_id);
> +        return -VIRTIO_CRYPTO_INVSESS;
> +    }
> +    if (sess->pending_close) {
> +        qemu_mutex_unlock(&lkcf->mutex);
> +        error_report("Session %" PRIu64 " is closing", op_info->session_id);
> +        return -VIRTIO_CRYPTO_INVSESS;
> +    }
> +    sess->in_use++;
> +    qemu_mutex_unlock(&lkcf->mutex);
> +
>      task = g_new0(CryptoDevLKCFTask, 1);
>      task->op_info = op_info;
>      task->cb = op_info->cb;
> @@ -606,8 +641,30 @@ static int cryptodev_lkcf_close_session(CryptoDevBackend *backend,
>      CryptoDevBackendLKCFSession *session;
>  
>      assert(session_id < MAX_SESSIONS && lkcf->sess[session_id]);
> +
> +    qemu_mutex_lock(&lkcf->mutex);
>      session = lkcf->sess[session_id];
> +
> +    /*
> +     * Mark session as pending close. New operations using this session
> +     * will be rejected. We hold the mutex until in_use becomes 0 to
> +     * prevent race conditions.
> +     */
> +    session->pending_close = true;
> +
> +    /*
> +     * Wait for all in-flight tasks using this session to complete.
> +     * The worker thread decrements in_use after task execution.
> +     */
> +    while (session->in_use > 0) {
> +        qemu_cond_wait(&lkcf->cond, &lkcf->mutex);
> +    }
> +
> +    /*
> +     * Now safe to remove session and free resources.
> +     */
>      lkcf->sess[session_id] = NULL;
> +    qemu_mutex_unlock(&lkcf->mutex);
>  
>      g_free(session->key);
>      g_free(session);
> -- 
> 2.43.0
> 

With regards,
Daniel
-- 
|: https://berrange.com       ~~        https://hachyderm.io/@berrange :|
|: https://libvirt.org          ~~          https://entangle-photo.org :|
|: https://pixelfed.art/berrange   ~~    https://fstop138.berrange.com :|
Re: [PATCH v2] backends/cryptodev-lkcf: fix use-after-free in session lifecycle
Posted by zhenwei pi 1 month, 2 weeks ago
Reviewed-by: zhenwei pi <zhenwei.pi@linux.dev>

On 4/16/26 14:19, Gonglei wrote:
> The cryptodev-lkcf backend had a race condition where session close
> could free a session while tasks using that session were still pending
> in the queue. This leads to use-after-free when the worker thread
> later accesses the freed session pointer.
> 
> Add reference counting (in_use) and pending_close flag to ensure:
> - New operations are rejected when a session is closing
> - Session close waits for all in-flight tasks to complete
> - No use-after-free can occur
> 
> Fixes: CVE-2026-6288
> Fixes: 39fff6f3e8 ("cryptodev: Add a lkcf-backend for cryptodev")
> Reported-by: Buzzy <buzzy0257@gmail.com>
> Signed-off-by: Gonglei <arei.gonglei@huawei.com>
> Tested-by: Buzzy <buzzy0257@gmail.com>
> ---
> Changes:
> 
> v2:
>   * moved sess->pending_close checking before @task allocated
>     in cryptodev_lkcf_operation().
> 
> ---
>   backends/cryptodev-lkcf.c | 59 ++++++++++++++++++++++++++++++++++++++-
>   1 file changed, 58 insertions(+), 1 deletion(-)
> 
> diff --git a/backends/cryptodev-lkcf.c b/backends/cryptodev-lkcf.c
> index 40c7bd3c5a..3a93c81372 100644
> --- a/backends/cryptodev-lkcf.c
> +++ b/backends/cryptodev-lkcf.c
> @@ -66,6 +66,9 @@ typedef struct CryptoDevBackendLKCFSession {
>       size_t keylen;
>       QCryptoAkCipherKeyType keytype;
>       QCryptoAkCipherOptions akcipher_opts;
> +    int in_use;  /* number of tasks currently using this session */
> +    /* session close requested, waiting for in_use to become 0 */
> +    bool pending_close;
>   } CryptoDevBackendLKCFSession;
>   
>   typedef struct CryptoDevLKCFTask CryptoDevLKCFTask;
> @@ -428,6 +431,18 @@ out:
>       if (key_id >= 0) {
>           keyctl_unlink(key_id, KCTL_KEY_RING);
>       }
> +
> +    /*
> +     * Decrement session in_use counter and signal if session is pending close.
> +     * This allows close_session to proceed after all tasks complete.
> +     */
> +    qemu_mutex_lock(&task->lkcf->mutex);
> +    task->sess->in_use--;
> +    if (task->sess->pending_close && task->sess->in_use == 0) {
> +        qemu_cond_broadcast(&task->lkcf->cond);
> +    }
> +    qemu_mutex_unlock(&task->lkcf->mutex);
> +
>       task->status = status;
>   
>       qemu_mutex_lock(&task->lkcf->rsp_mutex);
> @@ -486,12 +501,32 @@ static int cryptodev_lkcf_operation(
>           return -VIRTIO_CRYPTO_INVSESS;
>       }
>   
> -    sess = lkcf->sess[op_info->session_id];
>       if (algtype != QCRYPTODEV_BACKEND_ALGO_TYPE_ASYM) {
>           error_report("algtype not supported: %u", algtype);
>           return -VIRTIO_CRYPTO_NOTSUPP;
>       }
>   
> +    /*
> +     * Check if session is pending close and increment in_use counter
> +     * atomically under the mutex. This prevents the session from being
> +     * freed while a task is pending.
> +     */
> +    qemu_mutex_lock(&lkcf->mutex);
> +    sess = lkcf->sess[op_info->session_id];
> +    if (!sess) {
> +        qemu_mutex_unlock(&lkcf->mutex);
> +        error_report("Cannot find a valid session id: %" PRIu64 "",
> +                     op_info->session_id);
> +        return -VIRTIO_CRYPTO_INVSESS;
> +    }
> +    if (sess->pending_close) {
> +        qemu_mutex_unlock(&lkcf->mutex);
> +        error_report("Session %" PRIu64 " is closing", op_info->session_id);
> +        return -VIRTIO_CRYPTO_INVSESS;
> +    }
> +    sess->in_use++;
> +    qemu_mutex_unlock(&lkcf->mutex);
> +
>       task = g_new0(CryptoDevLKCFTask, 1);
>       task->op_info = op_info;
>       task->cb = op_info->cb;
> @@ -606,8 +641,30 @@ static int cryptodev_lkcf_close_session(CryptoDevBackend *backend,
>       CryptoDevBackendLKCFSession *session;
>   
>       assert(session_id < MAX_SESSIONS && lkcf->sess[session_id]);
> +
> +    qemu_mutex_lock(&lkcf->mutex);
>       session = lkcf->sess[session_id];
> +
> +    /*
> +     * Mark session as pending close. New operations using this session
> +     * will be rejected. We hold the mutex until in_use becomes 0 to
> +     * prevent race conditions.
> +     */
> +    session->pending_close = true;
> +
> +    /*
> +     * Wait for all in-flight tasks using this session to complete.
> +     * The worker thread decrements in_use after task execution.
> +     */
> +    while (session->in_use > 0) {
> +        qemu_cond_wait(&lkcf->cond, &lkcf->mutex);
> +    }
> +
> +    /*
> +     * Now safe to remove session and free resources.
> +     */
>       lkcf->sess[session_id] = NULL;
> +    qemu_mutex_unlock(&lkcf->mutex);
>   
>       g_free(session->key);
>       g_free(session);