* [bug report] SVM: nSVM: setup nested msr permission bitmap on nested state load
@ 2020-09-23 13:44 Dan Carpenter
2020-09-23 13:50 ` Maxim Levitsky
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2020-09-23 13:44 UTC (permalink / raw)
To: mlevitsk; +Cc: kvm
Hello Maxim Levitsky,
The patch 772b81bb2f9b: "SVM: nSVM: setup nested msr permission
bitmap on nested state load" from Aug 27, 2020, leads to the
following static checker warning:
arch/x86/kvm/svm/nested.c:1161 svm_set_nested_state()
warn: 'ctl' not released on lines: 1152.
arch/x86/kvm/svm/nested.c
1135 if (!(save->cr0 & X86_CR0_PG))
1136 goto out_free;
1137
1138 /*
1139 * All checks done, we can enter guest mode. L1 control fields
1140 * come from the nested save state. Guest state is already
1141 * in the registers, the save area of the nested state instead
1142 * contains saved L1 state.
1143 */
1144 copy_vmcb_control_area(&hsave->control, &svm->vmcb->control);
1145 hsave->save = *save;
1146
1147 svm->nested.vmcb = kvm_state->hdr.svm.vmcb_pa;
1148 load_nested_vmcb_control(svm, ctl);
1149 nested_prepare_vmcb_control(svm);
1150
1151 if (!nested_svm_vmrun_msrpm(svm))
1152 return -EINVAL;
goto out_free?
1153
1154 svm_set_gif(svm, !!(kvm_state->flags & KVM_STATE_NESTED_GIF_SET));
1155
1156 ret = 0;
1157 out_free:
1158 kfree(save);
1159 kfree(ctl);
1160
1161 return ret;
1162 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] SVM: nSVM: setup nested msr permission bitmap on nested state load
2020-09-23 13:44 [bug report] SVM: nSVM: setup nested msr permission bitmap on nested state load Dan Carpenter
@ 2020-09-23 13:50 ` Maxim Levitsky
2020-09-23 14:29 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Maxim Levitsky @ 2020-09-23 13:50 UTC (permalink / raw)
To: Dan Carpenter; +Cc: kvm
On Wed, 2020-09-23 at 16:44 +0300, Dan Carpenter wrote:
> Hello Maxim Levitsky,
>
> The patch 772b81bb2f9b: "SVM: nSVM: setup nested msr permission
> bitmap on nested state load" from Aug 27, 2020, leads to the
> following static checker warning:
>
> arch/x86/kvm/svm/nested.c:1161 svm_set_nested_state()
> warn: 'ctl' not released on lines: 1152.
>
> arch/x86/kvm/svm/nested.c
> 1135 if (!(save->cr0 & X86_CR0_PG))
> 1136 goto out_free;
> 1137
> 1138 /*
> 1139 * All checks done, we can enter guest mode. L1 control fields
> 1140 * come from the nested save state. Guest state is already
> 1141 * in the registers, the save area of the nested state instead
> 1142 * contains saved L1 state.
> 1143 */
> 1144 copy_vmcb_control_area(&hsave->control, &svm->vmcb->control);
> 1145 hsave->save = *save;
> 1146
> 1147 svm->nested.vmcb = kvm_state->hdr.svm.vmcb_pa;
> 1148 load_nested_vmcb_control(svm, ctl);
> 1149 nested_prepare_vmcb_control(svm);
> 1150
> 1151 if (!nested_svm_vmrun_msrpm(svm))
> 1152 return -EINVAL;
>
> goto out_free?
>
> 1153
> 1154 svm_set_gif(svm, !!(kvm_state->flags & KVM_STATE_NESTED_GIF_SET));
> 1155
> 1156 ret = 0;
> 1157 out_free:
> 1158 kfree(save);
> 1159 kfree(ctl);
> 1160
> 1161 return ret;
> 1162 }
>
> regards,
> dan carpenter
>
Which kernel tree is this?
This again seems to be the result of other commit
that made save, ctl to be dynamically allocated. I based my patch on the version
that allocates both on the stack so no freeing is needed.
As far as I know from a check I did about week ago, none of branches on
git://git.kernel.org/pub/scm/virt/kvm/kvm.git had that patch (that made save/ctr
be allocated dynamically).
Best regards,
Maxim Levitsky
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] SVM: nSVM: setup nested msr permission bitmap on nested state load
2020-09-23 13:50 ` Maxim Levitsky
@ 2020-09-23 14:29 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2020-09-23 14:29 UTC (permalink / raw)
To: Maxim Levitsky, Joerg Roedel; +Cc: kvm
On Wed, Sep 23, 2020 at 04:50:58PM +0300, Maxim Levitsky wrote:
> On Wed, 2020-09-23 at 16:44 +0300, Dan Carpenter wrote:
> > Hello Maxim Levitsky,
> >
> > The patch 772b81bb2f9b: "SVM: nSVM: setup nested msr permission
> > bitmap on nested state load" from Aug 27, 2020, leads to the
> > following static checker warning:
> >
> > arch/x86/kvm/svm/nested.c:1161 svm_set_nested_state()
> > warn: 'ctl' not released on lines: 1152.
> >
> > arch/x86/kvm/svm/nested.c
> > 1135 if (!(save->cr0 & X86_CR0_PG))
> > 1136 goto out_free;
> > 1137
> > 1138 /*
> > 1139 * All checks done, we can enter guest mode. L1 control fields
> > 1140 * come from the nested save state. Guest state is already
> > 1141 * in the registers, the save area of the nested state instead
> > 1142 * contains saved L1 state.
> > 1143 */
> > 1144 copy_vmcb_control_area(&hsave->control, &svm->vmcb->control);
> > 1145 hsave->save = *save;
> > 1146
> > 1147 svm->nested.vmcb = kvm_state->hdr.svm.vmcb_pa;
> > 1148 load_nested_vmcb_control(svm, ctl);
> > 1149 nested_prepare_vmcb_control(svm);
> > 1150
> > 1151 if (!nested_svm_vmrun_msrpm(svm))
> > 1152 return -EINVAL;
> >
> > goto out_free?
> >
> > 1153
> > 1154 svm_set_gif(svm, !!(kvm_state->flags & KVM_STATE_NESTED_GIF_SET));
> > 1155
> > 1156 ret = 0;
> > 1157 out_free:
> > 1158 kfree(save);
> > 1159 kfree(ctl);
> > 1160
> > 1161 return ret;
> > 1162 }
> >
> > regards,
> > dan carpenter
> >
> Which kernel tree is this?
>
> This again seems to be the result of other commit
> that made save, ctl to be dynamically allocated. I based my patch on the version
> that allocates both on the stack so no freeing is needed.
>
> As far as I know from a check I did about week ago, none of branches on
> git://git.kernel.org/pub/scm/virt/kvm/kvm.git had that patch (that made save/ctr
> be allocated dynamically).
This is from yesterday's linux-next. Look like a merge issue with
commit 6ccbd29ade0d ("KVM: SVM: nested: Don't allocate VMCB structures
on stack").
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-09-23 14:30 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2020-09-23 13:44 [bug report] SVM: nSVM: setup nested msr permission bitmap on nested state load Dan Carpenter
2020-09-23 13:50 ` Maxim Levitsky
2020-09-23 14:29 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox