| Message ID | 20211206112738.14893-1-lizhang@suse.de |
|---|---|
| State | New |
| Headers | show |
| Series | [1/1] kvm: Clear variables which may not be used | expand |
On Mon, Dec 06, 2021 at 12:27:38PM +0100, Li Zhang wrote: > The variables msi, route in kvm_irqchip_send_msi may be uninitialised > values in some cases. It's necessary to clear them. You say the patch is going to 'clear them' but.... > > Signed-off-by: Li Zhang <lizhang@suse.de> > --- > accel/kvm/kvm-all.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c > index eecd8031cf..bd50dc6b80 100644 > --- a/accel/kvm/kvm-all.c > +++ b/accel/kvm/kvm-all.c > @@ -1913,10 +1913,8 @@ static KVMMSIRoute *kvm_lookup_msi_route(KVMState *s, MSIMessage msg) > > int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg) > { > - struct kvm_msi msi; > - KVMMSIRoute *route; > - > if (kvm_direct_msi_allowed) { > + struct kvm_msi msi; ...but this is still an uninitialized declaration. > msi.address_lo = (uint32_t)msg.address; > msi.address_hi = msg.address >> 32; > msi.data = le32_to_cpu(msg.data); I guess the bug you were wanting to fix is that this code only initializes 5 out of 6 struct fields, before calling the ioctl. > @@ -1926,6 +1924,7 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg) > return kvm_vm_ioctl(s, KVM_SIGNAL_MSI, &msi); > } > > + KVMMSIRoute *route; This was initialized correctly before and didn't need moving > route = kvm_lookup_msi_route(s, msg); > if (!route) { > int virq; Regards, Daniel
On 12/6/21 12:39 PM, Daniel P. Berrangé wrote: > On Mon, Dec 06, 2021 at 12:27:38PM +0100, Li Zhang wrote: >> The variables msi, route in kvm_irqchip_send_msi may be uninitialised >> values in some cases. It's necessary to clear them. > You say the patch is going to 'clear them' but.... Ah, sorry for my bad. And this is not correct fix. > >> Signed-off-by: Li Zhang <lizhang@suse.de> >> --- >> accel/kvm/kvm-all.c | 5 ++--- >> 1 file changed, 2 insertions(+), 3 deletions(-) >> >> diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c >> index eecd8031cf..bd50dc6b80 100644 >> --- a/accel/kvm/kvm-all.c >> +++ b/accel/kvm/kvm-all.c >> @@ -1913,10 +1913,8 @@ static KVMMSIRoute *kvm_lookup_msi_route(KVMState *s, MSIMessage msg) >> >> int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg) >> { >> - struct kvm_msi msi; >> - KVMMSIRoute *route; >> - >> if (kvm_direct_msi_allowed) { >> + struct kvm_msi msi; > ...but this is still an uninitialized declaration. > >> msi.address_lo = (uint32_t)msg.address; >> msi.address_hi = msg.address >> 32; >> msi.data = le32_to_cpu(msg.data); > I guess the bug you were wanting to fix is that this code only > initializes 5 out of 6 struct fields, before calling the > ioctl. Yes, you are right. >> @@ -1926,6 +1924,7 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg) >> return kvm_vm_ioctl(s, KVM_SIGNAL_MSI, &msi); >> } >> >> + KVMMSIRoute *route; > This was initialized correctly before and didn't need moving > >> route = kvm_lookup_msi_route(s, msg); >> if (!route) { >> int virq; > Regards, > Daniel
diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c index eecd8031cf..bd50dc6b80 100644 --- a/accel/kvm/kvm-all.c +++ b/accel/kvm/kvm-all.c @@ -1913,10 +1913,8 @@ static KVMMSIRoute *kvm_lookup_msi_route(KVMState *s, MSIMessage msg) int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg) { - struct kvm_msi msi; - KVMMSIRoute *route; - if (kvm_direct_msi_allowed) { + struct kvm_msi msi; msi.address_lo = (uint32_t)msg.address; msi.address_hi = msg.address >> 32; msi.data = le32_to_cpu(msg.data); @@ -1926,6 +1924,7 @@ int kvm_irqchip_send_msi(KVMState *s, MSIMessage msg) return kvm_vm_ioctl(s, KVM_SIGNAL_MSI, &msi); } + KVMMSIRoute *route; route = kvm_lookup_msi_route(s, msg); if (!route) { int virq;
The variables msi, route in kvm_irqchip_send_msi may be uninitialised values in some cases. It's necessary to clear them. Signed-off-by: Li Zhang <lizhang@suse.de> --- accel/kvm/kvm-all.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-)