[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH 1/2] gnttab: remove guest_physmap_remove_page() call from gnttab_map_frame()


  • To: Julien Grall <julien@xxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Thu, 2 Dec 2021 11:12:56 +0100
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=hb6wXT7j+7j5WnsHGVg+Hd3o/WlwkCVs1TK7CN19H/4=; b=iGZcbnBwUy+bnGIUHX7rIIrp09XouVVK6LBZB0f9wfMN1H0GFdNAL0n0Ryqv/MIR61pewSdF17tbdaHe09FlawnW5tu2oHbRicgmOoHpf3JyoWXG9NrthwCPzrMcBToUA/ZDPL4JqAJasIVZy/QLWQbjvcru8eHpbO+/f2K+WLKp0VogoB1fqJpfDg+MoZODgNNVPYCDk2zll+LxeulW/9fQuUMDbj04tdr9wMqzORYJZulTcSm6SJgpuRpyY4wNomOpguzXCc6kXxdC4XShlC9SwvGiQsXg2DT9EFjsnZayxwWMy40SJZLhWROTHhyRvfH36jXNlf9DrzmRhXFC2g==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=PVct31ihfcaaiQr50VgN2WGFe1QTLBdhf9ZXuz9eT6pa2eVxvb18HM+680Px6czdJ3umF7Kk71XJk/hzX+5ol6tU1c7lpLSu1VB64L2ygZseRprBbTchwWpBGzSXoNhnsQsMNEXv8NOQmQ0YKWykr6coxH9QklC7dY22/Wg2/08vwCWcL3LANUGkZQvqFKDWETTH6uW6WH7wGclKsWMbnJZ7xRNIhVa/zrsyM+wze6/6NIv7xtsj/f6fhCaD2r5NqTW4T2cqvXI9IGNf/f0u7p7gqL31jR8o5pz3DblbcaC79R0OXufPiHnqbvuaQkIYRiognI/Erfo2cu6Kc+hGCA==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>, "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • Delivery-date: Thu, 02 Dec 2021 10:13:10 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

On 02.12.2021 10:09, Julien Grall wrote:
> Hi Jan,
> 
> On 13/09/2021 07:41, Jan Beulich wrote:
>> Without holding appropriate locks, attempting to remove a prior mapping
>> of the underlying page is pointless, as the same (or another) mapping
>> could be re-established by a parallel request on another vCPU. Move the
>> code to Arm's gnttab_set_frame_gfn(). Of course this new placement
>> doesn't improve things in any way as far as the security of grant status
>> frame mappings goes (see XSA-379). Proper locking would be needed here
>> to allow status frames to be mapped securely.
>>
>> In turn this then requires replacing the other use in
>> gnttab_unpopulate_status_frames(), which yet in turn requires adjusting
>> x86's gnttab_set_frame_gfn(). Note that with proper locking inside
>> guest_physmap_remove_page() combined with checking the GFN's mapping
>> there against the passed in MFN, there then is no issue with the
>> involved multiple gnttab_set_frame_gfn()-s potentially returning varying
>> values (due to a racing XENMAPSPACE_grant_table request).
>>
>> This, as a side effect, does away with gnttab_map_frame() having a local
>> variable "gfn" which shadows a function parameter of the same name.
>>
>> Together with XSA-379 this points out that XSA-255's addition to
>> gnttab_map_frame() was really useless.
>>
>> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> As discussed in the thread, I expect the Arm part to be simplified after 
> Oleksandr's series. So for the Arm part:
> 
> Acked-by: Julien Grall <jgrall@xxxxxxxxxx>

Thanks, but let me ask for a clarification: Explicitly just the Arm part,
or also the common code change? I ask because from the x86 side I already
have an ack by Roger, but strictly speaking that doesn't cover common
code.

Jan




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.