Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751418AbdILSyf (ORCPT ); Tue, 12 Sep 2017 14:54:35 -0400 Received: from smtp.ctxuk.citrix.com ([185.25.65.24]:22248 "EHLO SMTP.EU.CITRIX.COM" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751316AbdILSya (ORCPT ); Tue, 12 Sep 2017 14:54:30 -0400 X-IronPort-AV: E=Sophos;i="5.42,383,1500940800"; d="scan'208";a="52702060" Subject: Re: [Xen-devel] [PATCH 4/4] xen: select grant interface version To: Juergen Gross , , CC: References: <20170908144849.2958-1-jgross@suse.com> <20170908144849.2958-5-jgross@suse.com> From: Andrew Cooper Message-ID: Date: Tue, 12 Sep 2017 19:54:22 +0100 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.2.1 MIME-Version: 1.0 In-Reply-To: <20170908144849.2958-5-jgross@suse.com> Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit Content-Language: en-GB X-ClientProxiedBy: AMSPEX02CAS01.citrite.net (10.69.22.112) To AMSPEX02CL02.citrite.net (10.69.22.126) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 1365 Lines: 39 On 08/09/17 15:48, Juergen Gross wrote: > static void gnttab_request_version(void) > { > - int rc; > + long rc; > struct gnttab_set_version gsv; > > - gsv.version = 1; > + rc = HYPERVISOR_memory_op(XENMEM_maximum_ram_page, NULL); This hypercall is information leak and layering violation. Please can we avoid adding more dependence on its presence? (I'm got a proto-series which strips various corners off the hypervisor for attack surface reduction purposes, and this hypercall is one victim which is restricted to privileged domains only.) For translated guests, it is definitely not the right number to check. What matters is the maximum frame inside the translated guest, not on the host. For PV guests, I'm not sure what to suggest, but the result of XENMEM_maximum_ram_page isn't applicable. Xen's max_page can increase at runtime through memory hotplug, after which ballooning operations can leave Linux with a frame it wishes to grant which exceeds the limit calculated here. The more I look into this, the more of a mess it turns out to be. ~Andrew > + if (rc < 0 || !(rc >> 32)) > + gsv.version = 1; > + else > + gsv.version = 2; > + if (xen_gnttab_version >= 1 && xen_gnttab_version <= 2) > + gsv.version = xen_gnttab_version; > > rc = HYPERVISOR_grant_table_op(GNTTABOP_set_version, &gsv, 1); > if (rc == 0 && gsv.version == 2)