linux-kernel - RE: [PATCH v3] hyperv: root partition faults writing to VP ASSIST MSR PAGE

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 27 Jul 2021 17:23:10 +0000
From:   Sunil Muthuswamy <sunilmut@...rosoft.com>
To:     Praveen Kumar <kumarpraveen@...ux.microsoft.com>,
        "linux-hyperv@...r.kernel.org" <linux-hyperv@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
CC:     KY Srinivasan <kys@...rosoft.com>,
        Haiyang Zhang <haiyangz@...rosoft.com>,
        Stephen Hemminger <sthemmin@...rosoft.com>,
        "wei.liu@...nel.org" <wei.liu@...nel.org>,
        Dexuan Cui <decui@...rosoft.com>,
        "tglx@...utronix.de" <tglx@...utronix.de>,
        "mingo@...hat.com" <mingo@...hat.com>,
        "bp@...en8.de" <bp@...en8.de>, "x86@...nel.org" <x86@...nel.org>,
        "hpa@...or.com" <hpa@...or.com>,
        "viremana@...ux.microsoft.com" <viremana@...ux.microsoft.com>,
        "nunodasneves@...ux.microsoft.com" <nunodasneves@...ux.microsoft.com>
Subject: RE: [PATCH v3] hyperv: root partition faults writing to VP ASSIST MSR
 PAGE

> >>  static int hv_cpu_init(unsigned int cpu)
> >>  {
> >> +	union hv_vp_assist_msr_contents msr;
> >>  	struct hv_vp_assist_page **hvp = &hv_vp_assist_page[smp_processor_id()];
> >>  	int ret;
> >>
> >> @@ -54,27 +55,41 @@ static int hv_cpu_init(unsigned int cpu)
> >>  	if (!hv_vp_assist_page)
> >>  		return 0;
> >
> > Not related to this code, but I am not sure about the usefulness of this NULL check as
> > we have already accessed this pointer above. If it was NULL, things would already
> > blow up.
> >
> What I understood, hvp will point to "hv_vp_assist_page stack address + smp_processor_id()"
> So, we are good, and this NULL check is required, as in when we de-reference the location, later in the code, it may fault.
> Please do correct me if my understanding is wrong here. Thanks.
> 

'hv_vp_assist_page' comes from the heap, there is nothing on the stack there. As I
mentioned previously, if 'hv_vp_assist_page' was NULL, we would have already
crashed by now, as we are accessing it above. So, the check here is useless in my opinion.
But, since this is code that this patch doesn't touch, its fine to leave it as is for this patch.
I was just pointing it out.

- Sunil