Message ID | 20230306160016.4459-98-tzimmermann@suse.de (mailing list archive) |
---|---|
State | Superseded |
Headers | show |
Series | fbdev: Fix memory leak in option parsing | expand |
Hi Thomas, I love your patch! Yet something to improve: [auto build test ERROR on drm-misc/drm-misc-next] [cannot apply to deller-parisc/for-next staging/staging-testing staging/staging-next staging/staging-linus linus/master v6.3-rc1 next-20230306] [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-98-tzimmermann%40suse.de patch subject: [PATCH 97/99] fbdev/vt8623fb: Duplicate video-mode option string config: x86_64-randconfig-a016-20230306 (https://download.01.org/0day-ci/archive/20230307/202303070859.8Y5URjpT-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/819a7fd9e1404efc4f2140bcb4c7e39643b7e4ab 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 819a7fd9e1404efc4f2140bcb4c7e39643b7e4ab # 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/202303070859.8Y5URjpT-lkp@intel.com/ All errors (new ones prefixed by >>): >> drivers/video/fbdev/vt8623fb.c:938:4: error: 'break' statement not in loop or switch statement break; ^ drivers/video/fbdev/vt8623fb.c:940:4: error: 'break' statement not in loop or switch statement break; ^ 2 errors generated. vim +/break +938 drivers/video/fbdev/vt8623fb.c 924 925 if (fb_modesetting_disabled("vt8623fb")) 926 return -ENODEV; 927 928 #ifndef MODULE 929 if (fb_get_options("vt8623fb", &option)) 930 return -ENODEV; 931 932 if (option && *option) { 933 static char mode_option_buf[256]; 934 int ret; 935 936 ret = snprintf(mode_option_buf, sizeof(mode_option_buf), "%s", option); 937 if (WARN(ret < 0, "vt8623fb: ignoring invalid option, ret=%d\n", ret)) > 938 break; 939 if (WARN(ret >= sizeof(mode_option_buf), "vt8623fb: option too long\n")) 940 break; 941 mode_option = mode_option_buf; 942 } 943 #endif 944 945 pr_debug("vt8623fb: initializing\n"); 946 return pci_register_driver(&vt8623fb_pci_driver); 947 } 948
diff --git a/drivers/video/fbdev/vt8623fb.c b/drivers/video/fbdev/vt8623fb.c index 034333ee6e45..cbdca42d1708 100644 --- a/drivers/video/fbdev/vt8623fb.c +++ b/drivers/video/fbdev/vt8623fb.c @@ -929,8 +929,17 @@ static int __init vt8623fb_init(void) if (fb_get_options("vt8623fb", &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, "vt8623fb: ignoring invalid option, ret=%d\n", ret)) + break; + if (WARN(ret >= sizeof(mode_option_buf), "vt8623fb: option too long\n")) + break; + mode_option = mode_option_buf; + } #endif pr_debug("vt8623fb: initializing\n");
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/vt8623fb.c | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)