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);
}
}
