2022-09-21 18:00:19

by Dawei Li

[permalink] [raw]
Subject: [PATCH] drm/vmwgfx: Protect pin_user_pages with mmap_lock

This patch includes changes below:
1) pin_user_pages() is unsafe without protection of mmap_lock,
fix it by calling mmap_read_lock() & mmap_read_unlock().
2) fix & refactor the incorrect exception handling procedure in
vmw_mksstat_add_ioctl().

based-on branch: vmwgfx/drm-misc-fixes
based commit: d8a79c03054911c375a2252627a429c9bc4615b6

Signed-off-by: Dawei Li <[email protected]>
---
drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 24 +++++++++++++++---------
1 file changed, 15 insertions(+), 9 deletions(-)

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
index 2aceac7856e2..ec40a3364e0a 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
@@ -1020,9 +1020,9 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void *data,
const size_t num_pages_info = PFN_UP(arg->info_len);
const size_t num_pages_strs = PFN_UP(arg->strs_len);
long desc_len;
- long nr_pinned_stat;
- long nr_pinned_info;
- long nr_pinned_strs;
+ long nr_pinned_stat = 0;
+ long nr_pinned_info = 0;
+ long nr_pinned_strs = 0;
struct page *pages_stat[ARRAY_SIZE(pdesc->statPPNs)];
struct page *pages_info[ARRAY_SIZE(pdesc->infoPPNs)];
struct page *pages_strs[ARRAY_SIZE(pdesc->strsPPNs)];
@@ -1076,6 +1076,7 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void *data,

if (desc_len < 0) {
atomic_set(&dev_priv->mksstat_user_pids[slot], 0);
+ __free_page(page);
return -EFAULT;
}

@@ -1083,28 +1084,33 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void *data,
reset_ppn_array(pdesc->infoPPNs, ARRAY_SIZE(pdesc->infoPPNs));
reset_ppn_array(pdesc->strsPPNs, ARRAY_SIZE(pdesc->strsPPNs));

+ /* pin_user_pages() needs protection of mmap_lock */
+ mmap_read_lock(current->mm);
+
/* Pin mksGuestStat user pages and store those in the instance descriptor */
nr_pinned_stat = pin_user_pages(arg->stat, num_pages_stat, FOLL_LONGTERM, pages_stat, NULL);
if (num_pages_stat != nr_pinned_stat)
- goto err_pin_stat;
+ goto __err_pin_pages;

for (i = 0; i < num_pages_stat; ++i)
pdesc->statPPNs[i] = page_to_pfn(pages_stat[i]);

nr_pinned_info = pin_user_pages(arg->info, num_pages_info, FOLL_LONGTERM, pages_info, NULL);
if (num_pages_info != nr_pinned_info)
- goto err_pin_info;
+ goto __err_pin_pages;

for (i = 0; i < num_pages_info; ++i)
pdesc->infoPPNs[i] = page_to_pfn(pages_info[i]);

nr_pinned_strs = pin_user_pages(arg->strs, num_pages_strs, FOLL_LONGTERM, pages_strs, NULL);
if (num_pages_strs != nr_pinned_strs)
- goto err_pin_strs;
+ goto __err_pin_pages;

for (i = 0; i < num_pages_strs; ++i)
pdesc->strsPPNs[i] = page_to_pfn(pages_strs[i]);

+ mmap_read_unlock(current->mm);
+
/* Send the descriptor to the host via a hypervisor call. The mksGuestStat
pages will remain in use until the user requests a matching remove stats
or a stats reset occurs. */
@@ -1119,15 +1125,15 @@ int vmw_mksstat_add_ioctl(struct drm_device *dev, void *data,

return 0;

-err_pin_strs:
+__err_pin_pages:
+ mmap_read_unlock(current->mm);
+
if (nr_pinned_strs > 0)
unpin_user_pages(pages_strs, nr_pinned_strs);

-err_pin_info:
if (nr_pinned_info > 0)
unpin_user_pages(pages_info, nr_pinned_info);

-err_pin_stat:
if (nr_pinned_stat > 0)
unpin_user_pages(pages_stat, nr_pinned_stat);

--
2.25.1