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

Re: [XEN v2] Xen: Ensure "xenheap_bits - PAGE_SHIFT" can be used to shift "unsigned long"


  • To: Jan Beulich <jbeulich@xxxxxxxx>, Ayan Kumar Halder <ayan.kumar.halder@xxxxxxx>
  • From: Ayan Kumar Halder <ayankuma@xxxxxxx>
  • Date: Fri, 2 Dec 2022 09:30:10 +0000
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=amd.com; dmarc=pass action=none header.from=amd.com; dkim=pass header.d=amd.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=NFVRM6GqmR69Rkton10Je9G+TqBKtrsIIlgkNlTXFE8=; b=maqhqkVcoWRwaJPAnH7+g8LxkkUxZEshWNvYRc0vIll9JIvPS6jqJ0+2g0SxAXdB3scR1JNNcNje22axY7ruqJP+BUDoVVX7JEXKxETTokrpQH7lQdF+/SJT9a96YAvf3dq7CxD9C+WJvDGHeq6R0E+tROITebOM3Zi9s3BPeMNEcWrG/wp621kCZc7Ve6IioOxtJDA+4UUTkpc5JcM5JzG/h7Yl5lwyVkiAX7yI1sXZUwQCHsP+1l/4G1ZS1emrKNzAuWYetjQXCegyZKamQRBzfkh915XRWzHwjrWWeeOQMjlsk5/GDnv7T/muGFvtvlZU+122rvqXJk2SXByCMQ==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=aLt4ZD5WmQ9jkw/osf34dK0TYSoHGtV/aQItlLDUDkzFCbC1def+8urAfioDM8y0jYnGzCDiqzU5hToTyneHbz81IAep62yC6PGBeXZTNc/ihJPZdeMbefbHHGykllRJERx507bxEXrszoII/g4qxp6FKekZMmNemQuhuA60WbD3mg10D1NvmXDdPrVDsLc7+uuOfBtk3swfDcEuBClX2vZtNAD1wHdCfH0FwhjPJVRQaFZy5Nx4cnP5Rh/ZLh2nVr/w3M64cM42abJ9rZEkNgEbriY2/DacIkyhlfEX0aO0zd+oKuo58t/WAHuulY+AP7XF46OxsbAnoaeWaPRUlw==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=amd.com;
  • Cc: sstabellini@xxxxxxxxxx, stefanos@xxxxxxxxxx, julien@xxxxxxx, andrew.cooper3@xxxxxxxxxx, george.dunlap@xxxxxxxxxx, wl@xxxxxxx, xen-devel@xxxxxxxxxxxxxxxxxxxx
  • Delivery-date: Fri, 02 Dec 2022 09:30:39 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

Hi Jan,

On 02/12/2022 08:31, Jan Beulich wrote:
On 01.12.2022 19:11, Ayan Kumar Halder wrote:
Machine frame number (mfn) is used to represent the hardware page address.
This is an unsigned long variable. We need to check if it can hold the complete
range of hardware page addresses. To ensure this we check that the count of bits
represented by 'unsigned long' added to the bit index of page size, should be
less than the count of bits required to represent the maximum physical address.
I'm afraid I can't connect the description with ...

--- a/xen/common/page_alloc.c
+++ b/xen/common/page_alloc.c
@@ -2245,7 +2245,7 @@ void __init xenheap_max_mfn(unsigned long mfn)
  {
      ASSERT(!first_node_initialised);
      ASSERT(!xenheap_bits);
-    BUILD_BUG_ON(PADDR_BITS >= BITS_PER_LONG);
+    BUILD_BUG_ON((PADDR_BITS - PAGE_SHIFT) >= BITS_PER_LONG);
... the actual change made. Julien, when replying to v1, already gave
a clear hint what is relevant: The use of (xenheap_bits - PAGE_SHIFT)
in right hand operands of shift operators. As relevant is of course
the absence of uses directly as shift counts, which otherwise could
still be UB (and which iirc is why the adjustment by PAGE_SHIFT was
left out in the original check).

I could see the following uses of xenheap_bits in page_alloc.c

1. init_node_heap()

              (!xenheap_bits ||
               !((mfn + nr - 1) >> (xenheap_bits - PAGE_SHIFT))) )

               (!xenheap_bits ||
               !((mfn + needed - 1) >> (xenheap_bits - PAGE_SHIFT))) )

2. In alloc_xenheap_pages()

    if ( xenheap_bits && (memflags >> _MEMF_bits) > xenheap_bits )
        memflags &= ~MEMF_bits(~0U);
    if ( !(memflags >> _MEMF_bits) )
        memflags |= MEMF_bits(xenheap_bits);

From what I see, whenever "xenheap_bits" is used as a right hand operand of shift operator, it is always used as "(xenheap_bits - PAGE_SHIFT)".

So, is it correct to say this :-

Ensure (xenheap_bits - PAGE_SHIFT) can be used as a rhs operand of a shift operator

We want to ensure that "xenheap_bits - PAGE_SHIFT" is strictly less than the number of bits to represent unsigned long as it is used a rhs operand to shift mfn.

- Ayan


Jan



 


Rackspace

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