From: Longpeng <[email protected]>
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 <[email protected]>
---
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(-)
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
On 03/11/2021 16:00, Longpeng(Mike) wrote:
>
> From: Longpeng <[email protected]>
>
> 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 <[email protected]>
> ---
> 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 <[email protected]>
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.