[08/99] fbdev/arkfb: Duplicate video-mode option string

Message ID 20230306160016.4459-9-tzimmermann@suse.de
State New
Headers
Series fbdev: Fix memory leak in option parsing |

Commit Message

Thomas Zimmermann March 6, 2023, 3:58 p.m. UTC
  Assume that the driver does not own the option string or its substrings
and hence duplicate the option string for the video mode. The driver only
parses the option string once as part of module initialization, so use
a static buffer to store the duplicated mode option. Linux automatically
frees the memory upon releasing the module.

Done in preparation of constifying the option string.

Signed-off-by: Thomas Zimmermann <tzimmermann@suse.de>
---
 drivers/video/fbdev/arkfb.c | 13 +++++++++++--
 1 file changed, 11 insertions(+), 2 deletions(-)
  

Comments

kernel test robot March 6, 2023, 9:12 p.m. UTC | #1
Hi Thomas,

I love your patch! Yet something to improve:

[auto build test ERROR on drm-misc/drm-misc-next]
[also build test ERROR on linus/master v6.3-rc1 next-20230306]
[cannot apply to deller-parisc/for-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/Thomas-Zimmermann/lib-Add-option-iterator/20230307-000524
base:   git://anongit.freedesktop.org/drm/drm-misc drm-misc-next
patch link:    https://lore.kernel.org/r/20230306160016.4459-9-tzimmermann%40suse.de
patch subject: [PATCH 08/99] fbdev/arkfb: Duplicate video-mode option string
config: x86_64-randconfig-a016-20230306 (https://download.01.org/0day-ci/archive/20230307/202303070537.699fZDEm-lkp@intel.com/config)
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/f8a56fb51ff846d7daca02280ac0355e1a82264e
        git remote add linux-review https://github.com/intel-lab-lkp/linux
        git fetch --no-tags linux-review Thomas-Zimmermann/lib-Add-option-iterator/20230307-000524
        git checkout f8a56fb51ff846d7daca02280ac0355e1a82264e
        # 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=x86_64 olddefconfig
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=x86_64 SHELL=/bin/bash

If you fix the issue, kindly add following tag where applicable
| Reported-by: kernel test robot <lkp@intel.com>
| Link: https://lore.kernel.org/oe-kbuild-all/202303070537.699fZDEm-lkp@intel.com/

All errors (new ones prefixed by >>):

>> drivers/video/fbdev/arkfb.c:1205:4: error: 'continue' statement not in loop statement
                           continue;
                           ^
   drivers/video/fbdev/arkfb.c:1207:4: error: 'continue' statement not in loop statement
                           continue;
                           ^
   2 errors generated.


vim +/continue +1205 drivers/video/fbdev/arkfb.c

  1191	
  1192		if (fb_modesetting_disabled("arkfb"))
  1193			return -ENODEV;
  1194	
  1195	#ifndef MODULE
  1196		if (fb_get_options("arkfb", &option))
  1197			return -ENODEV;
  1198	
  1199		if (option && *option) {
  1200			static char mode_option_buf[256];
  1201			int ret;
  1202	
  1203			ret = snprintf(mode_option_buf, sizeof(mode_option_buf), "%s", option);
  1204			if (WARN(ret < 0, "arkfb: ignoring invalid option, ret=%d\n", ret))
> 1205				continue;
  1206			if (WARN(ret >= sizeof(mode_option_buf), "arkfb: option too long\n"))
  1207				continue;
  1208			mode_option = mode_option_buf;
  1209		}
  1210	#endif
  1211	
  1212		pr_debug("arkfb: initializing\n");
  1213		return pci_register_driver(&arkfb_pci_driver);
  1214	}
  1215
  

Patch

diff --git a/drivers/video/fbdev/arkfb.c b/drivers/video/fbdev/arkfb.c
index 60a96fdb5dd8..f83fcdaec7a0 100644
--- a/drivers/video/fbdev/arkfb.c
+++ b/drivers/video/fbdev/arkfb.c
@@ -1196,8 +1196,17 @@  static int __init arkfb_init(void)
 	if (fb_get_options("arkfb", &option))
 		return -ENODEV;
 
-	if (option && *option)
-		mode_option = option;
+	if (option && *option) {
+		static char mode_option_buf[256];
+		int ret;
+
+		ret = snprintf(mode_option_buf, sizeof(mode_option_buf), "%s", option);
+		if (WARN(ret < 0, "arkfb: ignoring invalid option, ret=%d\n", ret))
+			continue;
+		if (WARN(ret >= sizeof(mode_option_buf), "arkfb: option too long\n"))
+			continue;
+		mode_option = mode_option_buf;
+	}
 #endif
 
 	pr_debug("arkfb: initializing\n");