* [bug report] KVM: arm64: nv: Handle ERETA[AB] instructions
@ 2024-04-24 6:39 Dan Carpenter
2024-04-24 12:28 ` Marc Zyngier
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2024-04-24 6:39 UTC (permalink / raw)
To: maz; +Cc: kvmarm
Hello Marc Zyngier,
Commit 213b3d1ea161 ("KVM: arm64: nv: Handle ERETA[AB] instructions")
from Apr 19, 2024 (linux-next), leads to the following Smatch static
checker warning:
arch/arm64/kvm/emulate-nested.c:2209 kvm_emulate_nested_eret()
error: uninitialized symbol 'elr'.
arch/arm64/kvm/emulate-nested.c
2173 void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)
2174 {
2175 u64 spsr, elr, esr;
2176
2177 /*
2178 * Forward this trap to the virtual EL2 if the virtual
2179 * HCR_EL2.NV bit is set and this is coming from !EL2.
2180 */
2181 if (forward_traps(vcpu, HCR_NV))
2182 return;
2183
2184 /* Check for an ERETAx */
2185 esr = kvm_vcpu_get_esr(vcpu);
2186 if (esr_iss_is_eretax(esr) && !kvm_auth_eretax(vcpu, &elr)) {
2187 /*
2188 * Oh no, ERETAx failed to authenticate. If we have
2189 * FPACCOMBINE, deliver an exception right away. If we
2190 * don't, then let the mangled ELR value trickle down the
2191 * ERET handling, and the guest will have a little surprise.
2192 */
2193 if (kvm_has_pauth(vcpu->kvm, FPACCOMBINE)) {
2194 esr &= ESR_ELx_ERET_ISS_ERETA;
2195 esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_FPAC);
2196 kvm_inject_nested_sync(vcpu, esr);
2197 return;
2198 }
2199 }
2200
2201 preempt_disable();
2202 kvm_arch_vcpu_put(vcpu);
2203
2204 spsr = __vcpu_sys_reg(vcpu, SPSR_EL2);
2205 spsr = kvm_check_illegal_exception_return(vcpu, spsr);
2206 if (!esr_iss_is_eretax(esr))
2207 elr = __vcpu_sys_reg(vcpu, ELR_EL2);
elr is unitialized on else path
2208
--> 2209 trace_kvm_nested_eret(vcpu, elr, spsr);
2210
2211 /*
2212 * Note that the current exception level is always the virtual EL2,
2213 * since we set HCR_EL2.NV bit only when entering the virtual EL2.
2214 */
2215 *vcpu_pc(vcpu) = elr;
^^^
2216 *vcpu_cpsr(vcpu) = spsr;
2217
2218 kvm_arch_vcpu_load(vcpu, smp_processor_id());
2219 preempt_enable();
2220 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] KVM: arm64: nv: Handle ERETA[AB] instructions
2024-04-24 6:39 [bug report] KVM: arm64: nv: Handle ERETA[AB] instructions Dan Carpenter
@ 2024-04-24 12:28 ` Marc Zyngier
2024-04-24 12:47 ` Dan Carpenter
0 siblings, 1 reply; 4+ messages in thread
From: Marc Zyngier @ 2024-04-24 12:28 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kvmarm
Hi Dan,
On Wed, 24 Apr 2024 07:39:53 +0100,
Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> Hello Marc Zyngier,
>
> Commit 213b3d1ea161 ("KVM: arm64: nv: Handle ERETA[AB] instructions")
> from Apr 19, 2024 (linux-next), leads to the following Smatch static
> checker warning:
>
> arch/arm64/kvm/emulate-nested.c:2209 kvm_emulate_nested_eret()
> error: uninitialized symbol 'elr'.
>
> arch/arm64/kvm/emulate-nested.c
> 2173 void kvm_emulate_nested_eret(struct kvm_vcpu *vcpu)
> 2174 {
> 2175 u64 spsr, elr, esr;
> 2176
> 2177 /*
> 2178 * Forward this trap to the virtual EL2 if the virtual
> 2179 * HCR_EL2.NV bit is set and this is coming from !EL2.
> 2180 */
> 2181 if (forward_traps(vcpu, HCR_NV))
> 2182 return;
> 2183
> 2184 /* Check for an ERETAx */
> 2185 esr = kvm_vcpu_get_esr(vcpu);
> 2186 if (esr_iss_is_eretax(esr) && !kvm_auth_eretax(vcpu, &elr)) {
> 2187 /*
> 2188 * Oh no, ERETAx failed to authenticate. If we have
> 2189 * FPACCOMBINE, deliver an exception right away. If we
> 2190 * don't, then let the mangled ELR value trickle down the
> 2191 * ERET handling, and the guest will have a little surprise.
> 2192 */
> 2193 if (kvm_has_pauth(vcpu->kvm, FPACCOMBINE)) {
> 2194 esr &= ESR_ELx_ERET_ISS_ERETA;
> 2195 esr |= FIELD_PREP(ESR_ELx_EC_MASK, ESR_ELx_EC_FPAC);
> 2196 kvm_inject_nested_sync(vcpu, esr);
> 2197 return;
> 2198 }
> 2199 }
> 2200
> 2201 preempt_disable();
> 2202 kvm_arch_vcpu_put(vcpu);
> 2203
> 2204 spsr = __vcpu_sys_reg(vcpu, SPSR_EL2);
> 2205 spsr = kvm_check_illegal_exception_return(vcpu, spsr);
> 2206 if (!esr_iss_is_eretax(esr))
> 2207 elr = __vcpu_sys_reg(vcpu, ELR_EL2);
>
> elr is unitialized on else path
Not quite.
Look at line 2186, and realise that kvm_auth_eretax() does write to
elr by being passed a pointer to it.
I guess Smatch isn't capable of seeing through something being
assigned in another compilation unit.
The code *could* be written as:
if (!esr_iss_is_eretax(esr)) {
[...]
elr = vcpu_sys_reg(vcpu, ELR_EL2);
[...]
} else if (!kvm_auth_eretax(vcpu, &elr))
[...]
}
but that would pointlessly duplicate some of the logic, and I'm not
sure Smatch would grok that either.
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] KVM: arm64: nv: Handle ERETA[AB] instructions
2024-04-24 12:28 ` Marc Zyngier
@ 2024-04-24 12:47 ` Dan Carpenter
2024-04-24 13:02 ` Marc Zyngier
0 siblings, 1 reply; 4+ messages in thread
From: Dan Carpenter @ 2024-04-24 12:47 UTC (permalink / raw)
To: Marc Zyngier; +Cc: kvmarm
Crud. I should have seen that. That's my bad. Certainly don't change
the code to silence the warning.
The issue for Smatch is that it doesn't parse bitwise tests or pure
functions properly... I know *how* to fix it and it's not *complicated*
but it's quite a bit of code and I haven't gotten around to doing it
yet.
regards,
dan carpenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [bug report] KVM: arm64: nv: Handle ERETA[AB] instructions
2024-04-24 12:47 ` Dan Carpenter
@ 2024-04-24 13:02 ` Marc Zyngier
0 siblings, 0 replies; 4+ messages in thread
From: Marc Zyngier @ 2024-04-24 13:02 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kvmarm
On Wed, 24 Apr 2024 13:47:11 +0100,
Dan Carpenter <dan.carpenter@linaro.org> wrote:
>
> Crud. I should have seen that. That's my bad. Certainly don't change
> the code to silence the warning.
>
> The issue for Smatch is that it doesn't parse bitwise tests or pure
> functions properly... I know *how* to fix it and it's not *complicated*
> but it's quite a bit of code and I haven't gotten around to doing it
> yet.
No worries. I prefer getting the odd report for a false positive and
look into it rather than turn a blind eye. At least you have a path
for the tool to be improved, which is what matters.
Thanks,
M.
--
Without deviation from the norm, progress is not possible.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-04-24 13:02 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-04-24 6:39 [bug report] KVM: arm64: nv: Handle ERETA[AB] instructions Dan Carpenter
2024-04-24 12:28 ` Marc Zyngier
2024-04-24 12:47 ` Dan Carpenter
2024-04-24 13:02 ` Marc Zyngier
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).