* [PATCH v3 0/1] fix module sysfs patch defect @ 2024-09-06 11:57 Chunhui Li 2024-09-06 11:57 ` [PATCH v3 1/1] module: abort module loading when sysfs setup suffer errors Chunhui Li 0 siblings, 1 reply; 5+ messages in thread From: Chunhui Li @ 2024-09-06 11:57 UTC (permalink / raw) To: Luis Chamberlain, Matthias Brugger, AngeloGioacchino Del Regno Cc: linux-modules, linux-kernel, linux-arm-kernel, linux-mediatek, wsd_upstream, Chunhui Li Hi Maintainer, I have update the patch base on your feedback, please help review again. V3: -Fix initial value; -Remove unnecessary checks about mod->sects_attrs -Check sysfs_creat_* return value as error code -Reorder the reveral of successful operations before exiting We will use the information recorded in mod->sects_attrs for debugging. When a panic occurs in a .ko, we can use gdb to load the symbols of the .ko file along with the coredump, and use the text segment address and init.text segment address in mod->sects_attrs to debug. If there is an error in add_sysfs_setup and the .ko is still loaded and runs normally, when the ko suffer panic later, we would not be able to access the loading addresses of the ko, which would prevent us from being able to debug effectively. Therefore, it is desired that if there is an error during load module, it should prevent the ko from being inserted with insmod. This ensures that as long as the .ko is successfully loaded, all necessary information for debugging is available. Thanks for your time. Chunhui Li (1): module: abort module loading when sysfs setup suffer errors kernel/module/sysfs.c | 57 +++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 21 deletions(-) -- 2.45.2 ^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v3 1/1] module: abort module loading when sysfs setup suffer errors 2024-09-06 11:57 [PATCH v3 0/1] fix module sysfs patch defect Chunhui Li @ 2024-09-06 11:57 ` Chunhui Li 2024-09-06 23:26 ` Luis Chamberlain ` (2 more replies) 0 siblings, 3 replies; 5+ messages in thread From: Chunhui Li @ 2024-09-06 11:57 UTC (permalink / raw) To: Luis Chamberlain, Matthias Brugger, AngeloGioacchino Del Regno Cc: linux-modules, linux-kernel, linux-arm-kernel, linux-mediatek, wsd_upstream, Chunhui Li, Petr Pavlu, kernel test robot, Xion Wang When insmod a kernel module, if fails in add_notes_attrs or add_sysfs_attrs such as memory allocation fail, mod_sysfs_setup will still return success, but we can't access user interface on android device. Patch for make mod_sysfs_setup can check the error of add_notes_attrs and add_sysfs_attrs Acked-by: Luis Chamberlain <mcgrof@kernel.org> Reviewed-by: Petr Pavlu <petr.pavlu@suse.com> Reported-by: kernel test robot <lkp@intel.com> Closes: https://lore.kernel.org/oe-kbuild-all/202409010016.3XIFSmRA-lkp@intel.com/ Signed-off-by: Xion Wang <xion.wang@mediatek.com> Signed-off-by: Chunhui Li <chunhui.li@mediatek.com> --- kernel/module/sysfs.c | 57 +++++++++++++++++++++++++++---------------- 1 file changed, 36 insertions(+), 21 deletions(-) diff --git a/kernel/module/sysfs.c b/kernel/module/sysfs.c index 26efe1305c12..0e0a9137a273 100644 --- a/kernel/module/sysfs.c +++ b/kernel/module/sysfs.c @@ -69,12 +69,13 @@ static void free_sect_attrs(struct module_sect_attrs *sect_attrs) kfree(sect_attrs); } -static void add_sect_attrs(struct module *mod, const struct load_info *info) +static int add_sect_attrs(struct module *mod, const struct load_info *info) { unsigned int nloaded = 0, i, size[2]; struct module_sect_attrs *sect_attrs; struct module_sect_attr *sattr; struct bin_attribute **gattr; + int ret; /* Count loaded sections and allocate structures */ for (i = 0; i < info->hdr->e_shnum; i++) @@ -85,7 +86,7 @@ static void add_sect_attrs(struct module *mod, const struct load_info *info) size[1] = (nloaded + 1) * sizeof(sect_attrs->grp.bin_attrs[0]); sect_attrs = kzalloc(size[0] + size[1], GFP_KERNEL); if (!sect_attrs) - return; + return -ENOMEM; /* Setup section attributes. */ sect_attrs->grp.name = "sections"; @@ -103,8 +104,10 @@ static void add_sect_attrs(struct module *mod, const struct load_info *info) sattr->address = sec->sh_addr; sattr->battr.attr.name = kstrdup(info->secstrings + sec->sh_name, GFP_KERNEL); - if (!sattr->battr.attr.name) + if (!sattr->battr.attr.name) { + ret = -ENOMEM; goto out; + } sect_attrs->nsections++; sattr->battr.read = module_sect_read; sattr->battr.size = MODULE_SECT_READ_SIZE; @@ -113,13 +116,15 @@ static void add_sect_attrs(struct module *mod, const struct load_info *info) } *gattr = NULL; - if (sysfs_create_group(&mod->mkobj.kobj, §_attrs->grp)) + ret = sysfs_create_group(&mod->mkobj.kobj, §_attrs->grp); + if (ret) goto out; mod->sect_attrs = sect_attrs; - return; + return 0; out: free_sect_attrs(sect_attrs); + return ret; } static void remove_sect_attrs(struct module *mod) @@ -158,15 +163,12 @@ static void free_notes_attrs(struct module_notes_attrs *notes_attrs, kfree(notes_attrs); } -static void add_notes_attrs(struct module *mod, const struct load_info *info) +static int add_notes_attrs(struct module *mod, const struct load_info *info) { unsigned int notes, loaded, i; struct module_notes_attrs *notes_attrs; struct bin_attribute *nattr; - - /* failed to create section attributes, so can't create notes */ - if (!mod->sect_attrs) - return; + int ret; /* Count notes sections and allocate structures. */ notes = 0; @@ -176,12 +178,12 @@ static void add_notes_attrs(struct module *mod, const struct load_info *info) ++notes; if (notes == 0) - return; + return 0; notes_attrs = kzalloc(struct_size(notes_attrs, attrs, notes), GFP_KERNEL); if (!notes_attrs) - return; + return -ENOMEM; notes_attrs->notes = notes; nattr = ¬es_attrs->attrs[0]; @@ -201,19 +203,23 @@ static void add_notes_attrs(struct module *mod, const struct load_info *info) } notes_attrs->dir = kobject_create_and_add("notes", &mod->mkobj.kobj); - if (!notes_attrs->dir) + if (!notes_attrs->dir) { + ret = -ENOMEM; goto out; + } - for (i = 0; i < notes; ++i) - if (sysfs_create_bin_file(notes_attrs->dir, - ¬es_attrs->attrs[i])) + for (i = 0; i < notes; ++i) { + ret = sysfs_create_bin_file(notes_attrs->dir, ¬es_attrs->attrs[i]); + if (ret) goto out; + } mod->notes_attrs = notes_attrs; - return; + return 0; out: free_notes_attrs(notes_attrs, i); + return ret; } static void remove_notes_attrs(struct module *mod) @@ -223,9 +229,9 @@ static void remove_notes_attrs(struct module *mod) } #else /* !CONFIG_KALLSYMS */ -static inline void add_sect_attrs(struct module *mod, const struct load_info *info) { } +static inline int add_sect_attrs(struct module *mod, const struct load_info *info) { } static inline void remove_sect_attrs(struct module *mod) { } -static inline void add_notes_attrs(struct module *mod, const struct load_info *info) { } +static inline int add_notes_attrs(struct module *mod, const struct load_info *info) { } static inline void remove_notes_attrs(struct module *mod) { } #endif /* CONFIG_KALLSYMS */ @@ -385,11 +391,20 @@ int mod_sysfs_setup(struct module *mod, if (err) goto out_unreg_modinfo_attrs; - add_sect_attrs(mod, info); - add_notes_attrs(mod, info); + err = add_sect_attrs(mod, info); + if (err) + goto out_del_usage_links; + + err = add_notes_attrs(mod, info); + if (err) + goto out_unreg_sect_attrs; return 0; +out_unreg_sect_attrs: + remove_sect_attrs(mod); +out_del_usage_links: + del_usage_links(mod); out_unreg_modinfo_attrs: module_remove_modinfo_attrs(mod, -1); out_unreg_param: -- 2.45.2 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/1] module: abort module loading when sysfs setup suffer errors 2024-09-06 11:57 ` [PATCH v3 1/1] module: abort module loading when sysfs setup suffer errors Chunhui Li @ 2024-09-06 23:26 ` Luis Chamberlain 2024-09-07 12:55 ` kernel test robot 2024-09-07 22:19 ` kernel test robot 2 siblings, 0 replies; 5+ messages in thread From: Luis Chamberlain @ 2024-09-06 23:26 UTC (permalink / raw) To: Chunhui Li Cc: Matthias Brugger, AngeloGioacchino Del Regno, linux-modules, linux-kernel, linux-arm-kernel, linux-mediatek, wsd_upstream, Petr Pavlu, kernel test robot, Xion Wang On Fri, Sep 06, 2024 at 07:57:48PM +0800, Chunhui Li wrote: > When insmod a kernel module, if fails in add_notes_attrs or > add_sysfs_attrs such as memory allocation fail, mod_sysfs_setup > will still return success, but we can't access user interface > on android device. > > Patch for make mod_sysfs_setup can check the error of > add_notes_attrs and add_sysfs_attrs > > Acked-by: Luis Chamberlain <mcgrof@kernel.org> > Reviewed-by: Petr Pavlu <petr.pavlu@suse.com> > Reported-by: kernel test robot <lkp@intel.com> > Closes: https://lore.kernel.org/oe-kbuild-all/202409010016.3XIFSmRA-lkp@intel.com/ > Signed-off-by: Xion Wang <xion.wang@mediatek.com> > Signed-off-by: Chunhui Li <chunhui.li@mediatek.com> Please add a Fixes: tag. Luis ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/1] module: abort module loading when sysfs setup suffer errors 2024-09-06 11:57 ` [PATCH v3 1/1] module: abort module loading when sysfs setup suffer errors Chunhui Li 2024-09-06 23:26 ` Luis Chamberlain @ 2024-09-07 12:55 ` kernel test robot 2024-09-07 22:19 ` kernel test robot 2 siblings, 0 replies; 5+ messages in thread From: kernel test robot @ 2024-09-07 12:55 UTC (permalink / raw) To: Chunhui Li, Luis Chamberlain, Matthias Brugger, AngeloGioacchino Del Regno Cc: oe-kbuild-all, linux-modules, linux-kernel, linux-arm-kernel, linux-mediatek, wsd_upstream, Chunhui Li, Petr Pavlu, kernel test robot, Xion Wang Hi Chunhui, kernel test robot noticed the following build warnings: [auto build test WARNING on mcgrof/modules-next] [also build test WARNING on linus/master v6.11-rc6 next-20240906] [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/Chunhui-Li/module-abort-module-loading-when-sysfs-setup-suffer-errors/20240907-000002 base: https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git modules-next patch link: https://lore.kernel.org/r/20240906115748.5367-2-chunhui.li%40mediatek.com patch subject: [PATCH v3 1/1] module: abort module loading when sysfs setup suffer errors config: openrisc-defconfig (https://download.01.org/0day-ci/archive/20240907/202409072018.qfEzZbO7-lkp@intel.com/config) compiler: or1k-linux-gcc (GCC) 14.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240907/202409072018.qfEzZbO7-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202409072018.qfEzZbO7-lkp@intel.com/ All warnings (new ones prefixed by >>): kernel/module/sysfs.c: In function 'add_sect_attrs': >> kernel/module/sysfs.c:232:67: warning: no return statement in function returning non-void [-Wreturn-type] 232 | static inline int add_sect_attrs(struct module *mod, const struct load_info *info) { } | ^~~~~~~~~ kernel/module/sysfs.c: In function 'add_notes_attrs': kernel/module/sysfs.c:234:68: warning: no return statement in function returning non-void [-Wreturn-type] 234 | static inline int add_notes_attrs(struct module *mod, const struct load_info *info) { } | ^~~~~~~~~ vim +232 kernel/module/sysfs.c 230 231 #else /* !CONFIG_KALLSYMS */ > 232 static inline int add_sect_attrs(struct module *mod, const struct load_info *info) { } 233 static inline void remove_sect_attrs(struct module *mod) { } 234 static inline int add_notes_attrs(struct module *mod, const struct load_info *info) { } 235 static inline void remove_notes_attrs(struct module *mod) { } 236 #endif /* CONFIG_KALLSYMS */ 237 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v3 1/1] module: abort module loading when sysfs setup suffer errors 2024-09-06 11:57 ` [PATCH v3 1/1] module: abort module loading when sysfs setup suffer errors Chunhui Li 2024-09-06 23:26 ` Luis Chamberlain 2024-09-07 12:55 ` kernel test robot @ 2024-09-07 22:19 ` kernel test robot 2 siblings, 0 replies; 5+ messages in thread From: kernel test robot @ 2024-09-07 22:19 UTC (permalink / raw) To: Chunhui Li, Luis Chamberlain, Matthias Brugger, AngeloGioacchino Del Regno Cc: llvm, oe-kbuild-all, linux-modules, linux-kernel, linux-arm-kernel, linux-mediatek, wsd_upstream, Chunhui Li, Petr Pavlu, kernel test robot, Xion Wang Hi Chunhui, kernel test robot noticed the following build warnings: [auto build test WARNING on mcgrof/modules-next] [also build test WARNING on linus/master v6.11-rc6 next-20240906] [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/Chunhui-Li/module-abort-module-loading-when-sysfs-setup-suffer-errors/20240907-000002 base: https://git.kernel.org/pub/scm/linux/kernel/git/mcgrof/linux.git modules-next patch link: https://lore.kernel.org/r/20240906115748.5367-2-chunhui.li%40mediatek.com patch subject: [PATCH v3 1/1] module: abort module loading when sysfs setup suffer errors config: powerpc-asp8347_defconfig (https://download.01.org/0day-ci/archive/20240908/202409080534.y2m1URF1-lkp@intel.com/config) compiler: clang version 17.0.6 (https://github.com/llvm/llvm-project 6009708b4367171ccdbf4b5905cb6a803753fe18) reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240908/202409080534.y2m1URF1-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202409080534.y2m1URF1-lkp@intel.com/ All warnings (new ones prefixed by >>): >> kernel/module/sysfs.c:232:86: warning: non-void function does not return a value [-Wreturn-type] 232 | static inline int add_sect_attrs(struct module *mod, const struct load_info *info) { } | ^ kernel/module/sysfs.c:234:87: warning: non-void function does not return a value [-Wreturn-type] 234 | static inline int add_notes_attrs(struct module *mod, const struct load_info *info) { } | ^ 2 warnings generated. vim +232 kernel/module/sysfs.c 230 231 #else /* !CONFIG_KALLSYMS */ > 232 static inline int add_sect_attrs(struct module *mod, const struct load_info *info) { } 233 static inline void remove_sect_attrs(struct module *mod) { } 234 static inline int add_notes_attrs(struct module *mod, const struct load_info *info) { } 235 static inline void remove_notes_attrs(struct module *mod) { } 236 #endif /* CONFIG_KALLSYMS */ 237 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2024-09-07 22:20 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-09-06 11:57 [PATCH v3 0/1] fix module sysfs patch defect Chunhui Li 2024-09-06 11:57 ` [PATCH v3 1/1] module: abort module loading when sysfs setup suffer errors Chunhui Li 2024-09-06 23:26 ` Luis Chamberlain 2024-09-07 12:55 ` kernel test robot 2024-09-07 22:19 ` kernel test robot
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).