Re: [bug report] SVM: nSVM: setup nested msr permission bitmap on nested state load

Dan Carpenter
2020-09-23T14:30:05Z

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