diff mbox series

[v3,02/17] drm/vmwgfx: Fix frame-size warning in vmw_mksstat_add_ioctl

Message ID 20221021034400.542909-3-zack@kde.org (mailing list archive)
State New, archived
Headers show
Series drm/vmwgfx: fb, cursors and hashtable refactor | expand

Commit Message

Zack Rusin Oct. 21, 2022, 3:43 a.m. UTC 
From: Martin Krastev <krastevm@vmware.com>

Function vmw_mksstat_add_ioctl allocates three big arrays on stack.
That triggers frame-size [-Wframe-larger-than=] warning. Refactor
that function to use kmalloc_array instead.

Signed-off-by: Martin Krastev <krastevm@vmware.com>
Reviewed-by: Zack Rusin <zackr@vmware.com>
Reviewed-by: Maaz Mombasawala <mombasawalam@vmware.com>
Signed-off-by: Zack Rusin <zackr@vmware.com>
---
 drivers/gpu/drm/vmwgfx/vmwgfx_msg.c | 39 ++++++++++++++++++++---------
 1 file changed, 27 insertions(+), 12 deletions(-)

Comments

kernel test robot Oct. 21, 2022, 1:16 p.m. UTC | #1 
Hi Zack,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on drm-misc/drm-misc-next]
[also build test WARNING on drm/drm-next drm-tip/drm-tip linus/master v6.1-rc1 next-20221021]
[cannot apply to drm-exynos/exynos-drm-next drm-intel/for-linux-next]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch#_base_tree_information]

url:    https://github.com/intel-lab-lkp/linux/commits/Zack-Rusin/drm-vmwgfx-fb-cursors-and-hashtable-refactor/20221021-114807
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20221021034400.542909-3-zack%40kde.org
patch subject: [PATCH v3 02/17] drm/vmwgfx: Fix frame-size warning in vmw_mksstat_add_ioctl
config: i386-randconfig-a013
compiler: clang version 14.0.6 (https://github.com/llvm/llvm-project f28c006a5895fc0e329fe15fead81e37457cb1d1)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # https://github.com/intel-lab-lkp/linux/commit/4efe4964595c6d6a9d2307aab879ae752e4fd4ac
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Zack-Rusin/drm-vmwgfx-fb-cursors-and-hashtable-refactor/20221021-114807
        git checkout 4efe4964595c6d6a9d2307aab879ae752e4fd4ac
        # save the config file
        mkdir build_dir && cp config build_dir/.config
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=i386 SHELL=/bin/bash drivers/gpu/drm/vmwgfx/

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:1064:6: warning: variable 'page' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (!pages_stat)
               ^~~~~~~~~~~
   drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:1148:6: note: uninitialized use occurs here
           if (page)
               ^~~~
   drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:1064:2: note: remove the 'if' if its condition is always false
           if (!pages_stat)
           ^~~~~~~~~~~~~~~~
   drivers/gpu/drm/vmwgfx/vmwgfx_msg.c:1017:19: note: initialize the variable 'page' to silence this warning
           struct page *page;
                            ^
                             = NULL
   1 warning generated.


vim +1064 drivers/gpu/drm/vmwgfx/vmwgfx_msg.c

   995	
   996	/**
   997	 * vmw_mksstat_add_ioctl: Creates a single userspace-originating mksGuestStat
   998	 * instance descriptor and registers that with the hypervisor.
   999	 *
  1000	 * Create a hypervisor PFN mapping, containing a single mksGuestStat instance
  1001	 * descriptor and pin the corresponding userspace pages.
  1002	 *
  1003	 * @dev: Identifies the drm device.
  1004	 * @data: Pointer to the ioctl argument.
  1005	 * @file_priv: Identifies the caller; unused.
  1006	 * Return: Zero on success, negative error code on error.
  1007	 */
  1008	
  1009	int vmw_mksstat_add_ioctl(struct drm_device *dev, void *data,
  1010					struct drm_file *file_priv)
  1011	{
  1012		struct drm_vmw_mksstat_add_arg *arg =
  1013			(struct drm_vmw_mksstat_add_arg *) data;
  1014	
  1015		struct vmw_private *const dev_priv = vmw_priv(dev);
  1016	
  1017		struct page *page;
  1018		MKSGuestStatInstanceDescriptor *pdesc;
  1019		const size_t num_pages_stat = PFN_UP(arg->stat_len);
  1020		const size_t num_pages_info = PFN_UP(arg->info_len);
  1021		const size_t num_pages_strs = PFN_UP(arg->strs_len);
  1022		long desc_len;
  1023		long nr_pinned_stat;
  1024		long nr_pinned_info;
  1025		long nr_pinned_strs;
  1026		struct page **pages_stat = NULL;
  1027		struct page **pages_info = NULL;
  1028		struct page **pages_strs = NULL;
  1029		size_t i, slot;
  1030		int ret_err = -ENOMEM;
  1031	
  1032		arg->id = -1;
  1033	
  1034		if (!arg->stat || !arg->info || !arg->strs)
  1035			return -EINVAL;
  1036	
  1037		if (!arg->stat_len || !arg->info_len || !arg->strs_len)
  1038			return -EINVAL;
  1039	
  1040		if (!arg->description)
  1041			return -EINVAL;
  1042	
  1043		if (num_pages_stat > ARRAY_SIZE(pdesc->statPPNs) ||
  1044			num_pages_info > ARRAY_SIZE(pdesc->infoPPNs) ||
  1045			num_pages_strs > ARRAY_SIZE(pdesc->strsPPNs))
  1046			return -EINVAL;
  1047	
  1048		/* Find an available slot in the mksGuestStats user array and reserve it */
  1049		for (slot = 0; slot < ARRAY_SIZE(dev_priv->mksstat_user_pids); ++slot)
  1050			if (!atomic_cmpxchg(&dev_priv->mksstat_user_pids[slot], 0, MKSSTAT_PID_RESERVED))
  1051				break;
  1052	
  1053		if (slot == ARRAY_SIZE(dev_priv->mksstat_user_pids))
  1054			return -ENOSPC;
  1055	
  1056		BUG_ON(dev_priv->mksstat_user_pages[slot]);
  1057	
  1058		/* Allocate statically-sized temp arrays for pages -- too big to keep in frame */
  1059		pages_stat = (struct page **)kmalloc_array(
  1060			ARRAY_SIZE(pdesc->statPPNs) +
  1061			ARRAY_SIZE(pdesc->infoPPNs) +
  1062			ARRAY_SIZE(pdesc->strsPPNs), sizeof(*pages_stat), GFP_KERNEL);
  1063	
> 1064		if (!pages_stat)
  1065			goto err_nomem;
  1066	
  1067		pages_info = pages_stat + ARRAY_SIZE(pdesc->statPPNs);
  1068		pages_strs = pages_info + ARRAY_SIZE(pdesc->infoPPNs);
  1069	
  1070		/* Allocate a page for the instance descriptor */
  1071		page = alloc_page(GFP_KERNEL | __GFP_ZERO);
  1072	
  1073		if (!page)
  1074			goto err_nomem;
  1075	
  1076		/* Set up the instance descriptor */
  1077		pdesc = page_address(page);
  1078	
  1079		pdesc->reservedMBZ = 0;
  1080		pdesc->statStartVA = arg->stat;
  1081		pdesc->strsStartVA = arg->strs;
  1082		pdesc->statLength = arg->stat_len;
  1083		pdesc->infoLength = arg->info_len;
  1084		pdesc->strsLength = arg->strs_len;
  1085		desc_len = strncpy_from_user(pdesc->description, u64_to_user_ptr(arg->description),
  1086			ARRAY_SIZE(pdesc->description) - 1);
  1087	
  1088		if (desc_len < 0) {
  1089			ret_err = -EFAULT;
  1090			goto err_nomem;
  1091		}
  1092	
  1093		reset_ppn_array(pdesc->statPPNs, ARRAY_SIZE(pdesc->statPPNs));
  1094		reset_ppn_array(pdesc->infoPPNs, ARRAY_SIZE(pdesc->infoPPNs));
  1095		reset_ppn_array(pdesc->strsPPNs, ARRAY_SIZE(pdesc->strsPPNs));
  1096	
  1097		/* Pin mksGuestStat user pages and store those in the instance descriptor */
  1098		nr_pinned_stat = pin_user_pages(arg->stat, num_pages_stat, FOLL_LONGTERM, pages_stat, NULL);
  1099		if (num_pages_stat != nr_pinned_stat)
  1100			goto err_pin_stat;
  1101	
  1102		for (i = 0; i < num_pages_stat; ++i)
  1103			pdesc->statPPNs[i] = page_to_pfn(pages_stat[i]);
  1104	
  1105		nr_pinned_info = pin_user_pages(arg->info, num_pages_info, FOLL_LONGTERM, pages_info, NULL);
  1106		if (num_pages_info != nr_pinned_info)
  1107			goto err_pin_info;
  1108	
  1109		for (i = 0; i < num_pages_info; ++i)
  1110			pdesc->infoPPNs[i] = page_to_pfn(pages_info[i]);
  1111	
  1112		nr_pinned_strs = pin_user_pages(arg->strs, num_pages_strs, FOLL_LONGTERM, pages_strs, NULL);
  1113		if (num_pages_strs != nr_pinned_strs)
  1114			goto err_pin_strs;
  1115	
  1116		for (i = 0; i < num_pages_strs; ++i)
  1117			pdesc->strsPPNs[i] = page_to_pfn(pages_strs[i]);
  1118	
  1119		/* Send the descriptor to the host via a hypervisor call. The mksGuestStat
  1120		   pages will remain in use until the user requests a matching remove stats
  1121		   or a stats reset occurs. */
  1122		hypervisor_ppn_add((PPN64)page_to_pfn(page));
  1123	
  1124		dev_priv->mksstat_user_pages[slot] = page;
  1125		atomic_set(&dev_priv->mksstat_user_pids[slot], task_pgrp_vnr(current));
  1126	
  1127		arg->id = slot;
  1128	
  1129		DRM_DEV_INFO(dev->dev, "pid=%d arg.description='%.*s' id=%zu\n", current->pid, (int)desc_len, pdesc->description, slot);
  1130	
  1131		kfree(pages_stat);
  1132		return 0;
  1133	
  1134	err_pin_strs:
  1135		if (nr_pinned_strs > 0)
  1136			unpin_user_pages(pages_strs, nr_pinned_strs);
  1137	
  1138	err_pin_info:
  1139		if (nr_pinned_info > 0)
  1140			unpin_user_pages(pages_info, nr_pinned_info);
  1141	
  1142	err_pin_stat:
  1143		if (nr_pinned_stat > 0)
  1144			unpin_user_pages(pages_stat, nr_pinned_stat);
  1145	
  1146	err_nomem:
  1147		atomic_set(&dev_priv->mksstat_user_pids[slot], 0);
  1148		if (page)
  1149			__free_page(page);
  1150		kfree(pages_stat);
  1151	
  1152		return ret_err;
  1153	}
  1154
diff mbox series

Patch

diff --git a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
index 089046fa21be..a6cea35eaa01 100644
--- a/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
+++ b/drivers/gpu/drm/vmwgfx/vmwgfx_msg.c
@@ -1023,10 +1023,11 @@  int vmw_mksstat_add_ioctl(struct drm_device *dev, void *data,
 	long nr_pinned_stat;
 	long nr_pinned_info;
 	long nr_pinned_strs;
-	struct page *pages_stat[ARRAY_SIZE(pdesc->statPPNs)];
-	struct page *pages_info[ARRAY_SIZE(pdesc->infoPPNs)];
-	struct page *pages_strs[ARRAY_SIZE(pdesc->strsPPNs)];
+	struct page **pages_stat = NULL;
+	struct page **pages_info = NULL;
+	struct page **pages_strs = NULL;
 	size_t i, slot;
+	int ret_err = -ENOMEM;
 
 	arg->id = -1;
 
@@ -1054,13 +1055,23 @@  int vmw_mksstat_add_ioctl(struct drm_device *dev, void *data,
 
 	BUG_ON(dev_priv->mksstat_user_pages[slot]);
 
+	/* Allocate statically-sized temp arrays for pages -- too big to keep in frame */
+	pages_stat = (struct page **)kmalloc_array(
+		ARRAY_SIZE(pdesc->statPPNs) +
+		ARRAY_SIZE(pdesc->infoPPNs) +
+		ARRAY_SIZE(pdesc->strsPPNs), sizeof(*pages_stat), GFP_KERNEL);
+
+	if (!pages_stat)
+		goto err_nomem;
+
+	pages_info = pages_stat + ARRAY_SIZE(pdesc->statPPNs);
+	pages_strs = pages_info + ARRAY_SIZE(pdesc->infoPPNs);
+
 	/* Allocate a page for the instance descriptor */
 	page = alloc_page(GFP_KERNEL | __GFP_ZERO);
 
-	if (!page) {
-		atomic_set(&dev_priv->mksstat_user_pids[slot], 0);
-		return -ENOMEM;
-	}
+	if (!page)
+		goto err_nomem;
 
 	/* Set up the instance descriptor */
 	pdesc = page_address(page);
@@ -1075,9 +1086,8 @@  int vmw_mksstat_add_ioctl(struct drm_device *dev, void *data,
 		ARRAY_SIZE(pdesc->description) - 1);
 
 	if (desc_len < 0) {
-		atomic_set(&dev_priv->mksstat_user_pids[slot], 0);
-		__free_page(page);
-		return -EFAULT;
+		ret_err = -EFAULT;
+		goto err_nomem;
 	}
 
 	reset_ppn_array(pdesc->statPPNs, ARRAY_SIZE(pdesc->statPPNs));
@@ -1118,6 +1128,7 @@  int vmw_mksstat_add_ioctl(struct drm_device *dev, void *data,
 
 	DRM_DEV_INFO(dev->dev, "pid=%d arg.description='%.*s' id=%zu\n", current->pid, (int)desc_len, pdesc->description, slot);
 
+	kfree(pages_stat);
 	return 0;
 
 err_pin_strs:
@@ -1132,9 +1143,13 @@  int vmw_mksstat_add_ioctl(struct drm_device *dev, void *data,
 	if (nr_pinned_stat > 0)
 		unpin_user_pages(pages_stat, nr_pinned_stat);
 
+err_nomem:
 	atomic_set(&dev_priv->mksstat_user_pids[slot], 0);
-	__free_page(page);
-	return -ENOMEM;
+	if (page)
+		__free_page(page);
+	kfree(pages_stat);
+
+	return ret_err;
 }
 
 /**