lkml.org 
[lkml]   [2018]   [Mar]   [14]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH v3 4/7] x86/hyper-v: allocate and use Virtual Processor Assist Pages
On Fri, 9 Mar 2018, Vitaly Kuznetsov wrote:
> @@ -198,6 +218,12 @@ static int hv_cpu_die(unsigned int cpu)
> struct hv_reenlightenment_control re_ctrl;
> unsigned int new_cpu;
>
> + if (hv_vp_assist_page && hv_vp_assist_page[cpu]) {
> + wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0);
> + vfree(hv_vp_assist_page[cpu]);
> + hv_vp_assist_page[cpu] = NULL;

So this is freed before the CPU is actually dead. And this runs in
preemtible context. Is the wrmsrl(HV_X64_MSR_VP_ASSIST_PAGE, 0); enough to
prevent eventual users of the assist page on the outgoing CPU from
accessing it?

> if (hv_reenlightenment_cb == NULL)
> return 0;
>
> @@ -241,6 +267,13 @@ void hyperv_init(void)
> if (!hv_vp_index)
> return;
>
> + hv_vp_assist_page = kcalloc(num_possible_cpus(),
> + sizeof(*hv_vp_assist_page), GFP_KERNEL);
> + if (!hv_vp_assist_page) {
> + ms_hyperv.hints &= ~HV_X64_ENLIGHTENED_VMCS_RECOMMENDED;
> + return;
> + }
> +
> if (cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "x86/hyperv_init:online",
> hv_cpu_init, hv_cpu_die) < 0)
> goto free_vp_index;

Shouldn't you free hv_vp_assist_page in the error path?

> +extern struct hv_vp_assist_page **hv_vp_assist_page;
> +
> +static inline struct hv_vp_assist_page *hv_get_vp_assist_page(unsigned int cpu)
> +{
> + return hv_vp_assist_page[cpu];

Shouldn't that check hv_vp_assist_page != NULL?

Thanks,

tglx

\
 
 \ /
  Last update: 2018-03-14 16:16    [W:0.170 / U:0.248 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site