All of lore.kernel.org
 help / color / mirror / Atom feed
From: Markus Armbruster <armbru@redhat.com>
To: qemu-devel@nongnu.org
Cc: Bibo Mao <maobibo@loongson.cn>, Song Gao <gaosong@loongson.cn>
Subject: Bad error handling in loongarch's kvm_arch_init_vcpu(), need advice
Date: Wed, 12 Mar 2025 09:39:40 +0100	[thread overview]
Message-ID: <87wmcumylv.fsf@pond.sub.org> (raw)

scripts/coccinelle/error-use-after-free.cocci led me to
target/loongarch/kvm/kvm.c:

    int kvm_arch_init_vcpu(CPUState *cs)
    {
        uint64_t val;
        int ret;
        Error *local_err = NULL;

        ret = 0;
        qemu_add_vm_change_state_handler(kvm_loongarch_vm_stage_change, cs);

        if (!kvm_get_one_reg(cs, KVM_REG_LOONGARCH_DEBUG_INST, &val)) {
            brk_insn = val;
        }

        ret = kvm_cpu_check_lsx(cs, &local_err);
        if (ret < 0) {
            error_report_err(local_err);

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

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

        }

        ret = kvm_cpu_check_lasx(cs, &local_err);

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

Two possible fixes:

1. If continuing after kvm_cpu_check_lasx() failure 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 to return ret.

What is the correct fix?

        if (ret < 0) {
            error_report_err(local_err);

Likewise.

        }

        ret = kvm_cpu_check_lbt(cs, &local_err);
        if (ret < 0) {
            error_report_err(local_err);

Likewise.

        }

        ret = kvm_cpu_check_pmu(cs, &local_err);
        if (ret < 0) {
            error_report_err(local_err);

Likewise.

        }

        ret = kvm_cpu_check_pv_features(cs, &local_err);
        if (ret < 0) {
            error_report_err(local_err);

Best to do the same here.

        }

        return ret;
    }



             reply	other threads:[~2025-03-12  8:40 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-12  8:39 Markus Armbruster [this message]
2025-03-12  8:59 ` Bad error handling in loongarch's kvm_arch_init_vcpu(), need advice Paolo Bonzini
2025-03-12  9:13   ` bibo mao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87wmcumylv.fsf@pond.sub.org \
    --to=armbru@redhat.com \
    --cc=gaosong@loongson.cn \
    --cc=maobibo@loongson.cn \
    --cc=qemu-devel@nongnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.