On Thu, Jan 05, 2023 at 12:38:30PM -0800, Vishal Annapurve wrote:

On Thu, Dec 1, 2022 at 10:20 PM Chao Peng [off-list ref] wrote:

quoted

+#ifdef CONFIG_HAVE_KVM_RESTRICTED_MEM
+static bool restrictedmem_range_is_valid(struct kvm_memory_slot *slot,
+                                        pgoff_t start, pgoff_t end,
+                                        gfn_t *gfn_start, gfn_t *gfn_end)
+{
+       unsigned long base_pgoff = slot->restricted_offset >> PAGE_SHIFT;
+
+       if (start > base_pgoff)
+               *gfn_start = slot->base_gfn + start - base_pgoff;

There should be a check for overflow here in case start is a very big
value. Additional check can look like:
if (start >= base_pgoff + slot->npages)
       return false;

quoted

+       else
+               *gfn_start = slot->base_gfn;
+
+       if (end < base_pgoff + slot->npages)
+               *gfn_end = slot->base_gfn + end - base_pgoff;

If "end" is smaller than base_pgoff, this can cause overflow and
return the range as valid. There should be additional check:
if (end < base_pgoff)
         return false;

Thanks! Both are good catches. The improved code:

static bool restrictedmem_range_is_valid(struct kvm_memory_slot *slot,
					 pgoff_t start, pgoff_t end,
					 gfn_t *gfn_start, gfn_t *gfn_end)
{
	unsigned long base_pgoff = slot->restricted_offset >> PAGE_SHIFT;

	if (start >= base_pgoff + slot->npages)
		return false;
	else if (start <= base_pgoff)
		*gfn_start = slot->base_gfn;
	else
		*gfn_start = start - base_pgoff + slot->base_gfn;

	if (end <= base_pgoff)
		return false;
	else if (end >= base_pgoff + slot->npages)
		*gfn_end = slot->base_gfn + slot->npages;
	else
		*gfn_end = end - base_pgoff + slot->base_gfn;

	if (*gfn_start >= *gfn_end)
		return false;

	return true;
}

Thanks,
Chao

quoted

+       else
+               *gfn_end = slot->base_gfn + slot->npages;
+
+       if (*gfn_start >= *gfn_end)
+               return false;
+
+       return true;
+}
+