[PATCH] virt: fsl_hypervisor: bound memcpy ioctl ranges

[PATCH] virt: fsl_hypervisor: bound memcpy ioctl ranges

[Yousef Alhouseen]

FSL_HV_IOCTL_MEMCPY takes the byte count from userspace as a u64, but
ioctl_memcpy() stored the derived page count in an unsigned int and the
remaining byte count in a uint32_t.

Large ranges can therefore wrap the page count before the pages and S/G
arrays are allocated. A wrapped zero page count can leave only the
alignment padding allocated for the S/G list before the function writes
sg_list[0]. The get_user_pages_fast() page-count argument is an int, so
larger values cannot be passed to it safely in any case.

Reject local addresses that cannot fit in unsigned long, calculate the
page count in 64 bits, cap it to INT_MAX, and allocate the aligned S/G
list with saturated size helpers. Keep the remaining byte count as u64 so
accepted ranges are described consistently.

Signed-off-by: Yousef Alhouseen <alhouseenyousef@gmail.com>
---
 drivers/virt/fsl_hypervisor.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)

diff --git a/drivers/virt/fsl_hypervisor.c b/drivers/virt/fsl_hypervisor.c
index f472c9480..bb01f9daf 100644
--- a/drivers/virt/fsl_hypervisor.c
+++ b/drivers/virt/fsl_hypervisor.c
@@ -153,13 +153,15 @@ static long ioctl_memcpy(struct fsl_hv_ioctl_memcpy __user *p)
 	struct fh_sg_list *sg_list = NULL;
 
 	unsigned int num_pages;
+	u64 num_pages64;
 	unsigned long lb_offset; /* Offset within a page of the local buffer */
+	size_t sg_size;
 
 	unsigned int i;
 	long ret = 0;
 	int num_pinned = 0; /* return value from get_user_pages_fast() */
 	phys_addr_t remote_paddr; /* The next address in the remote buffer */
-	uint32_t count; /* The number of bytes left to copy */
+	u64 count; /* The number of bytes left to copy */
 
 	/* Get the parameters from the user */
 	if (copy_from_user(&param, p, sizeof(struct fsl_hv_ioctl_memcpy)))
@@ -214,11 +216,22 @@ static long ioctl_memcpy(struct fsl_hv_ioctl_memcpy __user *p)
 	 * equal to the number of entries in the S/G list that we give to the
 	 * hypervisor.
 	 */
+	if (param.local_vaddr > ULONG_MAX)
+		return -EINVAL;
+
 	lb_offset = param.local_vaddr & (PAGE_SIZE - 1);
 	if (param.count == 0 ||
 	    param.count > U64_MAX - lb_offset - PAGE_SIZE + 1)
 		return -EINVAL;
-	num_pages = (param.count + lb_offset + PAGE_SIZE - 1) >> PAGE_SHIFT;
+	num_pages64 = (param.count + lb_offset + PAGE_SIZE - 1) >> PAGE_SHIFT;
+	if (num_pages64 > INT_MAX)
+		return -EINVAL;
+	num_pages = num_pages64;
+
+	sg_size = size_add(array_size(num_pages, sizeof(*sg_list)),
+			   sizeof(*sg_list) - 1);
+	if (sg_size == SIZE_MAX)
+		return -EINVAL;
 
 	/* Allocate the buffers we need */
 
@@ -236,8 +249,7 @@ static long ioctl_memcpy(struct fsl_hv_ioctl_memcpy __user *p)
 	 * sg_list is the list of fh_sg_list objects that we pass to the
 	 * hypervisor.
 	 */
-	sg_list_unaligned = kmalloc(num_pages * sizeof(struct fh_sg_list) +
-		sizeof(struct fh_sg_list) - 1, GFP_KERNEL);
+	sg_list_unaligned = kmalloc(sg_size, GFP_KERNEL);
 	if (!sg_list_unaligned) {
 		pr_debug("fsl-hv: could not allocate S/G list\n");
 		ret = -ENOMEM;
-- 
2.54.0