| 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
| ||
|
Message-ID: <779dcfe2-0eb6-97ea-f33c-5725939b00f8@amazon.com>
Date: Fri, 15 Oct 2021 16:49:24 +0300
From: "Paraschiv, Andra-Irina" <andraprs@...zon.com>
To: "Longpeng(Mike)" <longpeng2@...wei.com>
CC: <arei.gonglei@...wei.com>, <gregkh@...uxfoundation.org>,
<kamal@...onical.com>, <pbonzini@...hat.com>,
<sgarzare@...hat.com>, <stefanha@...hat.com>,
<vkuznets@...hat.com>, <linux-kernel@...r.kernel.org>,
<ne-devel-upstream@...zon.com>, <lexnv@...zon.com>,
<alcioa@...zon.com>
Subject: Re: [PATCH v3 2/4] nitro_enclaves: Sanity check physical memory
regions during merging
On 09/10/2021 04:32, Longpeng(Mike) wrote:
>
> From: Longpeng <longpeng2@...wei.com>
>
> Sanity check the physical memory regions during the merge of contiguous
> regions. Thus we can test the physical memory regions setup logic
> individually, including the error cases coming from the sanity checks.
>
> Signed-off-by: Longpeng <longpeng2@...wei.com>
> ---
> Changes v2 -> v3:
> - update the commit title and commit message. [Andra]
> - add comments before the function definition. [Andra]
> - remove 'inline' attribute of ne_sanity_check_phys_mem_region. [Andra]
> - leave a blank line before return. [Andra]
> - move sanity check in ne_merge_phys_contig_memory_regions to
> the beginning of the function. [Andra]
> - double sanity checking after the merge of physical contiguous
> memory regions has been completed. [Andra]
> ---
> drivers/virt/nitro_enclaves/ne_misc_dev.c | 79 ++++++++++++++++++++-----------
> 1 file changed, 52 insertions(+), 27 deletions(-)
>
> diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c b/drivers/virt/nitro_enclaves/ne_misc_dev.c
> index eea53e9..a8fa56b 100644
> --- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
> +++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
> @@ -836,6 +836,36 @@ static int ne_sanity_check_user_mem_region_page(struct ne_enclave *ne_enclave,
> }
>
> /**
> + * ne_sanity_check_phys_mem_region() - Sanity check the start address and the size
> + * of a physical memory region.
> + * @phys_mem_region_paddr : Physical start address of the region to be sanity checked.
> + * @phys_mem_region_size : Length of the region to be sanity checked.
> + *
> + * Return:
Please add this just before "Return", similar to other defined functions
(e.g. [1]):
* Context: Process context. This function is called with the ne_enclave
mutex held.
* Return:
......
[1]
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/drivers/virt/nitro_enclaves/ne_misc_dev.c#n725
> + * * 0 on success.
> + * * Negative return value on failure.
> + */
> +static int ne_sanity_check_phys_mem_region(u64 phys_mem_region_paddr,
> + u64 phys_mem_region_size)
> +{
> + if (phys_mem_region_size & (NE_MIN_MEM_REGION_SIZE - 1)) {
> + dev_err_ratelimited(ne_misc_dev.this_device,
> + "Physical mem region size is not multiple of 2 MiB\n");
> +
> + return -EINVAL;
> + }
> +
> + if (!IS_ALIGNED(phys_mem_region_paddr, NE_MIN_MEM_REGION_SIZE)) {
> + dev_err_ratelimited(ne_misc_dev.this_device,
> + "Physical mem region address is not 2 MiB aligned\n");
> +
> + return -EINVAL;
> + }
> +
> + return 0;
> +}
> +
> +/**
> * ne_merge_phys_contig_memory_regions() - Add a memory region and merge the adjacent
> * regions if they are physical contiguous.
> * @regions : Private data associated with the physical contiguous memory regions.
> @@ -843,23 +873,30 @@ static int ne_sanity_check_user_mem_region_page(struct ne_enclave *ne_enclave,
> * @page_size : Length of the region to be added.
> *
> * Return:
> - * * No return value.
> + * * 0 on success.
> + * * Negative return value on failure.
> */
> -static void
> +static int
> ne_merge_phys_contig_memory_regions(struct phys_contig_mem_regions *regions,
> u64 page_paddr, u64 page_size)
> {
> - /* Physical contiguous, just merge */
> + int rc = 0;
> +
> + rc = ne_sanity_check_phys_mem_region(page_paddr, page_size);
> + if (rc < 0)
> + return rc;
> +
> if (regions->num &&
> (regions->region[regions->num - 1].end + 1) == page_paddr) {
> + /* Physical contiguous, just merge */
Can move this comment before the "if (...)" line.
Thanks,
Andra
> regions->region[regions->num - 1].end += page_size;
> -
> - return;
> + } else {
> + regions->region[regions->num].start = page_paddr;
> + regions->region[regions->num].end = page_paddr + page_size - 1;
> + regions->num++;
> }
>
> - regions->region[regions->num].start = page_paddr;
> - regions->region[regions->num].end = page_paddr + page_size - 1;
> - regions->num++;
> + return 0;
> }
>
> /**
> @@ -940,9 +977,11 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
> if (rc < 0)
> goto put_pages;
>
> - ne_merge_phys_contig_memory_regions(phys_contig_mem_regions,
> - page_to_phys(ne_mem_region->pages[i]),
> - page_size(ne_mem_region->pages[i]));
> + rc = ne_merge_phys_contig_memory_regions(phys_contig_mem_regions,
> + page_to_phys(ne_mem_region->pages[i]),
> + page_size(ne_mem_region->pages[i]));
> + if (rc < 0)
> + goto put_pages;
>
> memory_size += page_size(ne_mem_region->pages[i]);
>
> @@ -965,23 +1004,9 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
> u64 phys_region_addr = range->start;
> u64 phys_region_size = range_len(range);
>
> - if (phys_region_size & (NE_MIN_MEM_REGION_SIZE - 1)) {
> - dev_err_ratelimited(ne_misc_dev.this_device,
> - "Physical mem region size is not multiple of 2 MiB\n");
> -
> - rc = -EINVAL;
> -
> - goto put_pages;
> - }
> -
> - if (!IS_ALIGNED(phys_region_addr, NE_MIN_MEM_REGION_SIZE)) {
> - dev_err_ratelimited(ne_misc_dev.this_device,
> - "Physical mem region address is not 2 MiB aligned\n");
> -
> - rc = -EINVAL;
> -
> + rc = ne_sanity_check_phys_mem_region(phys_region_addr, phys_region_size);
> + if (rc < 0)
> goto put_pages;
> - }
> }
>
> ne_mem_region->memory_size = mem_region.memory_size;
> --
> 1.8.3.1
>
Amazon Development Center (Romania) S.R.L. registered office: 27A Sf. Lazar Street, UBC5, floor 2, Iasi, Iasi County, 700045, Romania. Registered in Romania. Registration number J22/2621/2005.
Powered by blists - more mailing lists