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

["Daniel P. Berrangé" <berrange@redhat.com>]

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 :|