Bad error handling in cryptodev_lkcf_execute_task(), need advice

[Markus Armbruster <armbru@redhat.com>]

scripts/coccinelle/error-use-after-free.cocci led me to this function:

    static void cryptodev_lkcf_execute_task(CryptoDevLKCFTask *task)
    {
        CryptoDevBackendLKCFSession *session = task->sess;
        CryptoDevBackendAsymOpInfo *asym_op_info;
        bool kick = false;
        int ret, status, op_code = task->op_info->op_code;
        size_t p8info_len;
        g_autofree uint8_t *p8info = NULL;
        Error *local_error = NULL;
        key_serial_t key_id = INVALID_KEY_ID;
        char op_desc[64];
        g_autoptr(QCryptoAkCipher) akcipher = NULL;

        /**
         * We only offload private key session:
         * 1. currently, the Linux kernel can only accept public key wrapped
         * with X.509 certificates, but unfortunately the cost of making a
         * ceritificate with public key is too expensive.
         * 2. generally, public key related compution is fast, just compute it with
         * thread-pool.
         */
        if (session->keytype == QCRYPTO_AK_CIPHER_KEY_TYPE_PRIVATE) {
            if (qcrypto_akcipher_export_p8info(&session->akcipher_opts,
                                               session->key, session->keylen,
                                               &p8info, &p8info_len,
                                               &local_error) != 0 ||
                cryptodev_lkcf_set_op_desc(&session->akcipher_opts, op_desc,
                                           sizeof(op_desc), &local_error) != 0) {
                error_report_err(local_error);

Reporting an error, but continue anyway.  This is suspicious.

Note for later: @local_error is now non-null.

            } else {
                key_id = add_key(KCTL_KEY_TYPE_PKEY, "lkcf-backend-priv-key",
                                 p8info, p8info_len, KCTL_KEY_RING);
            }
        }

        if (key_id < 0) {
            if (!qcrypto_akcipher_supports(&session->akcipher_opts)) {
                status = -VIRTIO_CRYPTO_NOTSUPP;
                goto out;
            }
            akcipher = qcrypto_akcipher_new(&session->akcipher_opts,
                                            session->keytype,
                                            session->key, session->keylen,
                                            &local_error);

Passing non-null @local_error to qcrypto_akcipher_new().  This is wrong.
When qcrypto_akcipher_new() fails and passes &local_error to
error_setg(), error_setv()'s assertion will fail.

Two possible fixes:

1. If continuing after cryptodev_lkcf_set_op_desc() is correct, we need
to clear @local_error there.  Since it's not actually an error then, we
should almost certainly not use error_report_err() there.  *Maybe*
warn_report_err().

2. If continuing is wrong, we probably need set @status (to what?) and
goto out.

What is the correct fix?

            if (!akcipher) {
                status = -VIRTIO_CRYPTO_ERR;
                goto out;
            }
        }

    [...]

    out:
        if (key_id >= 0) {
            keyctl_unlink(key_id, KCTL_KEY_RING);
        }
        task->status = status;

        qemu_mutex_lock(&task->lkcf->rsp_mutex);
        if (QSIMPLEQ_EMPTY(&task->lkcf->responses)) {
            kick = true;
        }
        QSIMPLEQ_INSERT_TAIL(&task->lkcf->responses, task, queue);
        qemu_mutex_unlock(&task->lkcf->rsp_mutex);

        if (kick) {
            eventfd_write(task->lkcf->eventfd, 1);
        }
    }