| 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
| ||
|
Date: Sun, 7 Nov 2021 15:07:20 +0200
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 v4 2/4] nitro_enclaves: Sanity check physical memory
regions during merging
On 03/11/2021 16:00, 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 v3 -> v4:
> - add missing "Context" in the comments. [Andra]
> - move the comment in ne_merge_phys_contig_memory_regions() before
> the "if (...)". [Andra]
>
> 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 | 77 +++++++++++++++++++++----------
> 1 file changed, 52 insertions(+), 25 deletions(-)
Reviewed-by: Andra Paraschiv <andraprs@...zon.com>
You can add in the v5 of the patch series this line after the
"Signed-off-by" line, to track the already reviewed patches.
Thanks,
Andra
>
> diff --git a/drivers/virt/nitro_enclaves/ne_misc_dev.c b/drivers/virt/nitro_enclaves/ne_misc_dev.c
> index b7d2116..3741ae7 100644
> --- a/drivers/virt/nitro_enclaves/ne_misc_dev.c
> +++ b/drivers/virt/nitro_enclaves/ne_misc_dev.c
> @@ -836,6 +836,37 @@ 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.
> + *
> + * Context: Process context. This function is called with the ne_enclave mutex held.
> + * Return:
> + * * 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 physically contiguous.
> * @phys_contig_regions : Private data associated with the contiguous physical memory regions.
> @@ -843,23 +874,31 @@ static int ne_sanity_check_user_mem_region_page(struct ne_enclave *ne_enclave,
> * @page_size : Length of the region to be added.
> *
> * Context: Process context. This function is called with the ne_enclave mutex held.
> + * Return:
> + * * 0 on success.
> + * * Negative return value on failure.
> */
> -static void
> +static int
> ne_merge_phys_contig_memory_regions(struct ne_phys_contig_mem_regions *phys_contig_regions,
> u64 page_paddr, u64 page_size)
> {
> unsigned long num = phys_contig_regions->num;
> + int rc = 0;
> +
> + rc = ne_sanity_check_phys_mem_region(page_paddr, page_size);
> + if (rc < 0)
> + return rc;
>
> /* Physically contiguous, just merge */
> if (num && (phys_contig_regions->regions[num - 1].end + 1) == page_paddr) {
> phys_contig_regions->regions[num - 1].end += page_size;
> -
> - return;
> + } else {
> + phys_contig_regions->regions[num].start = page_paddr;
> + phys_contig_regions->regions[num].end = page_paddr + page_size - 1;
> + phys_contig_regions->num++;
> }
>
> - phys_contig_regions->regions[num].start = page_paddr;
> - phys_contig_regions->regions[num].end = page_paddr + page_size - 1;
> - phys_contig_regions->num++;
> + return 0;
> }
>
> /**
> @@ -938,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]);
>
> @@ -962,23 +1003,9 @@ static int ne_set_user_memory_region_ioctl(struct ne_enclave *ne_enclave,
> u64 phys_region_addr = phys_contig_mem_regions.regions[i].start;
> u64 phys_region_size = range_len(&phys_contig_mem_regions.regions[i]);
>
> - 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