[PATCH v1] binfmt_misc: fix crash when load/unload module

[PATCH v1] binfmt_misc: fix crash when load/unload module

[Tong Zhang]

We should unregister the table upon module unload otherwise something
horrible will happen when we load binfmt_misc module again. Also note
that we should keep value returned by register_sysctl_mount_point() and
release it later, otherwise it will leak.

reproduce:
modprobe binfmt_misc
modprobe -r binfmt_misc
modprobe binfmt_misc
modprobe -r binfmt_misc
modprobe binfmt_misc

[   18.032038] Call Trace:
[   18.032108]  <TASK>
[   18.032169]  dump_stack_lvl+0x34/0x44
[   18.032273]  __register_sysctl_table+0x6f4/0x720
[   18.032397]  ? preempt_count_sub+0xf/0xb0
[   18.032508]  ? 0xffffffffc0040000
[   18.032600]  init_misc_binfmt+0x2d/0x1000 [binfmt_misc]
[   18.042520] binfmt_misc: Failed to create fs/binfmt_misc sysctl mount point
modprobe: can't load module binfmt_misc (kernel/fs/binfmt_misc.ko): Cannot allocate memory
[   18.063549] binfmt_misc: Failed to create fs/binfmt_misc sysctl mount point
[   18.204779] BUG: unable to handle page fault for address: fffffbfff8004802

Fixes: 3ba442d5331f ("fs: move binfmt_misc sysctl to its own file")
Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 fs/binfmt_misc.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index ddea6acbddde..614aedb8ab2e 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -817,12 +817,16 @@ static struct file_system_type bm_fs_type = {
 };
 MODULE_ALIAS_FS("binfmt_misc");
 
+static struct ctl_table_header *binfmt_misc_header;
+
 static int __init init_misc_binfmt(void)
 {
 	int err = register_filesystem(&bm_fs_type);
 	if (!err)
 		insert_binfmt(&misc_format);
-	if (!register_sysctl_mount_point("fs/binfmt_misc")) {
+
+	binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");
+	if (!binfmt_misc_header) {
 		pr_warn("Failed to create fs/binfmt_misc sysctl mount point");
 		return -ENOMEM;
 	}
@@ -831,6 +835,7 @@ static int __init init_misc_binfmt(void)
 
 static void __exit exit_misc_binfmt(void)
 {
+	unregister_sysctl_table(binfmt_misc_header);
 	unregister_binfmt(&misc_format);
 	unregister_filesystem(&bm_fs_type);
 }
-- 
2.25.1

Re: [PATCH v1] binfmt_misc: fix crash when load/unload module

[Christian Brauner]

On Sun, Jan 23, 2022 at 04:33:41PM -0800, Tong Zhang wrote:
> We should unregister the table upon module unload otherwise something
> horrible will happen when we load binfmt_misc module again. Also note
> that we should keep value returned by register_sysctl_mount_point() and
> release it later, otherwise it will leak.
> 
> reproduce:
> modprobe binfmt_misc
> modprobe -r binfmt_misc
> modprobe binfmt_misc
> modprobe -r binfmt_misc
> modprobe binfmt_misc
> 
> [   18.032038] Call Trace:
> [   18.032108]  <TASK>
> [   18.032169]  dump_stack_lvl+0x34/0x44
> [   18.032273]  __register_sysctl_table+0x6f4/0x720
> [   18.032397]  ? preempt_count_sub+0xf/0xb0
> [   18.032508]  ? 0xffffffffc0040000
> [   18.032600]  init_misc_binfmt+0x2d/0x1000 [binfmt_misc]
> [   18.042520] binfmt_misc: Failed to create fs/binfmt_misc sysctl mount point
> modprobe: can't load module binfmt_misc (kernel/fs/binfmt_misc.ko): Cannot allocate memory
> [   18.063549] binfmt_misc: Failed to create fs/binfmt_misc sysctl mount point
> [   18.204779] BUG: unable to handle page fault for address: fffffbfff8004802
> 
> Fixes: 3ba442d5331f ("fs: move binfmt_misc sysctl to its own file")
> Signed-off-by: Tong Zhang <ztong0001@gmail.com>
> ---
>  fs/binfmt_misc.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
> index ddea6acbddde..614aedb8ab2e 100644
> --- a/fs/binfmt_misc.c
> +++ b/fs/binfmt_misc.c
> @@ -817,12 +817,16 @@ static struct file_system_type bm_fs_type = {
>  };
>  MODULE_ALIAS_FS("binfmt_misc");
>  
> +static struct ctl_table_header *binfmt_misc_header;
> +
>  static int __init init_misc_binfmt(void)
>  {
>  	int err = register_filesystem(&bm_fs_type);
>  	if (!err)
>  		insert_binfmt(&misc_format);
> -	if (!register_sysctl_mount_point("fs/binfmt_misc")) {
> +
> +	binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");
> +	if (!binfmt_misc_header) {

The fix itself is obviously needed.

However, afaict the previous patch introduced another bug and this patch
right here doesn't fix it either.

Namely, if you set CONFIG_SYSCTL=n and CONFIG_BINFMT_MISC={y,m}, then
register_sysctl_mount_point() will return NULL causing modprobe
binfmt_misc to fail. However, before 3ba442d5331f ("fs: move binfmt_misc
sysctl to its own file") loading binfmt_misc would've succeeded even if
fs/binfmt_misc wasn't created in kernel/sysctl.c. Afaict, that goes for
both CONFIG_SYSCTL={y,n} since even in the CONFIG_SYSCTL=y case the
kernel would've moved on if creating the sysctl header would've failed.
And that makes sense since binfmt_misc is mountable wherever, not just
at fs/binfmt_misc.

All that indicates that the correct fix here would be to simply:

binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");

without checking for an error. That should fully restore the old
behavior.

>  		pr_warn("Failed to create fs/binfmt_misc sysctl mount point");
>  		return -ENOMEM;
>  	}
> @@ -831,6 +835,7 @@ static int __init init_misc_binfmt(void)
>  
>  static void __exit exit_misc_binfmt(void)
>  {
> +	unregister_sysctl_table(binfmt_misc_header);
>  	unregister_binfmt(&misc_format);
>  	unregister_filesystem(&bm_fs_type);
>  }
> -- 
> 2.25.1
> 

Re: [PATCH v1] binfmt_misc: fix crash when load/unload module

[kernel test robot]

Hi Tong,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.17-rc1 next-20220124]
[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]

url:    https://github.com/0day-ci/linux/commits/Tong-Zhang/binfmt_misc-fix-crash-when-load-unload-module/20220124-083500
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dd81e1c7d5fb126e5fbc5c9e334d7b3ec29a16a0
config: riscv-randconfig-r001-20220123 (https://download.01.org/0day-ci/archive/20220124/202201241937.i9KSsyAj-lkp@intel.com/config)
compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 7b3d30728816403d1fd73cc5082e9fb761262bce)
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
        # install riscv cross compiling tool for clang build
        # apt-get install binutils-riscv64-linux-gnu
        # https://github.com/0day-ci/linux/commit/d649008f3214eb4d94760873831ef5e53c292976
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Tong-Zhang/binfmt_misc-fix-crash-when-load-unload-module/20220124-083500
        git checkout d649008f3214eb4d94760873831ef5e53c292976
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 O=build_dir ARCH=riscv SHELL=/bin/bash

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

All errors (new ones prefixed by >>):

>> fs/binfmt_misc.c:828:21: error: incompatible pointer types assigning to 'struct ctl_table_header *' from 'struct sysctl_header *' [-Werror,-Wincompatible-pointer-types]
           binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");
                              ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   1 error generated.


vim +828 fs/binfmt_misc.c

   821	
   822	static int __init init_misc_binfmt(void)
   823	{
   824		int err = register_filesystem(&bm_fs_type);
   825		if (!err)
   826			insert_binfmt(&misc_format);
   827	
 > 828		binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");
   829		if (!binfmt_misc_header) {
   830			pr_warn("Failed to create fs/binfmt_misc sysctl mount point");
   831			return -ENOMEM;
   832		}
   833		return 0;
   834	}
   835	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

Re: [PATCH v1] binfmt_misc: fix crash when load/unload module

[kernel test robot]

Hi Tong,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.17-rc1 next-20220124]
[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]

url:    https://github.com/0day-ci/linux/commits/Tong-Zhang/binfmt_misc-fix-crash-when-load-unload-module/20220124-083500
base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git dd81e1c7d5fb126e5fbc5c9e334d7b3ec29a16a0
config: arm-randconfig-c002-20220124 (https://download.01.org/0day-ci/archive/20220124/202201242006.cqM8NznF-lkp@intel.com/config)
compiler: arm-linux-gnueabi-gcc (GCC) 11.2.0
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/0day-ci/linux/commit/d649008f3214eb4d94760873831ef5e53c292976
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Tong-Zhang/binfmt_misc-fix-crash-when-load-unload-module/20220124-083500
        git checkout d649008f3214eb4d94760873831ef5e53c292976
        # save the config file to linux build tree
        mkdir build_dir
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-11.2.0 make.cross O=build_dir ARCH=arm SHELL=/bin/bash

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

All errors (new ones prefixed by >>):

   fs/binfmt_misc.c: In function 'init_misc_binfmt':
>> fs/binfmt_misc.c:828:28: error: assignment to 'struct ctl_table_header *' from incompatible pointer type 'struct sysctl_header *' [-Werror=incompatible-pointer-types]
     828 |         binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");
         |                            ^
   cc1: some warnings being treated as errors


vim +828 fs/binfmt_misc.c

   821	
   822	static int __init init_misc_binfmt(void)
   823	{
   824		int err = register_filesystem(&bm_fs_type);
   825		if (!err)
   826			insert_binfmt(&misc_format);
   827	
 > 828		binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");
   829		if (!binfmt_misc_header) {
   830			pr_warn("Failed to create fs/binfmt_misc sysctl mount point");
   831			return -ENOMEM;
   832		}
   833		return 0;
   834	}
   835	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[PATCH v2 0/2] Fix regression on binfmt_misc

[Tong Zhang]

This patch series fixes a couple of issues introduced in the previous
binfmt_misc refactor. Please see more information below.
Thanks,
- Tong

Changes since v1:

  - removed check from binfmt_misc to restore old behavior per Christian's
    comment
  - modified return type of register_sysctl_mount_point to fix CE

Tong Zhang (2):
  binfmt_misc: fix crash when load/unload module
  sysctl: fix return type to make compiler happy

 fs/binfmt_misc.c       | 8 ++++----
 include/linux/sysctl.h | 2 +-
 2 files changed, 5 insertions(+), 5 deletions(-)

-- 
2.25.1

[PATCH v2 1/2] binfmt_misc: fix crash when load/unload module

[Tong Zhang]

We should unregister the table upon module unload otherwise something
horrible will happen when we load binfmt_misc module again. Also note
that we should keep value returned by register_sysctl_mount_point() and
release it later, otherwise it will leak.
Also, per Christian's comment, to fully restore the old behavior that
won't break userspace the check(binfmt_misc_header) should be
eliminated.

reproduce:
modprobe binfmt_misc
modprobe -r binfmt_misc
modprobe binfmt_misc
modprobe -r binfmt_misc
modprobe binfmt_misc

[   18.032038] Call Trace:
[   18.032108]  <TASK>
[   18.032169]  dump_stack_lvl+0x34/0x44
[   18.032273]  __register_sysctl_table+0x6f4/0x720
[   18.032397]  ? preempt_count_sub+0xf/0xb0
[   18.032508]  ? 0xffffffffc0040000
[   18.032600]  init_misc_binfmt+0x2d/0x1000 [binfmt_misc]
[   18.042520] binfmt_misc: Failed to create fs/binfmt_misc sysctl mount point
modprobe: can't load module binfmt_misc (kernel/fs/binfmt_misc.ko): Cannot allocate memory
[   18.063549] binfmt_misc: Failed to create fs/binfmt_misc sysctl mount point
[   18.204779] BUG: unable to handle page fault for address: fffffbfff8004802

Fixes: 3ba442d5331f ("fs: move binfmt_misc sysctl to its own file")
Co-developed-by: Christian Brauner<brauner@kernel.org>
Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 fs/binfmt_misc.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index ddea6acbddde..c07f35719ee3 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -817,20 +817,20 @@ static struct file_system_type bm_fs_type = {
 };
 MODULE_ALIAS_FS("binfmt_misc");
 
+static struct ctl_table_header *binfmt_misc_header;
+
 static int __init init_misc_binfmt(void)
 {
 	int err = register_filesystem(&bm_fs_type);
 	if (!err)
 		insert_binfmt(&misc_format);
-	if (!register_sysctl_mount_point("fs/binfmt_misc")) {
-		pr_warn("Failed to create fs/binfmt_misc sysctl mount point");
-		return -ENOMEM;
-	}
+	binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");
 	return 0;
 }
 
 static void __exit exit_misc_binfmt(void)
 {
+	unregister_sysctl_table(binfmt_misc_header);
 	unregister_binfmt(&misc_format);
 	unregister_filesystem(&bm_fs_type);
 }
-- 
2.25.1

[PATCH v2 2/2] sysctl: fix return type to make compiler happy

[Tong Zhang]

When CONFIG_SYSCTL=n and CONFIG_BINFMT_MISC={y,m}, compiler will
complain due to return type not matching. Fix the return type in
register_sysctl_mount_point() to make compiler happy

fs/binfmt_misc.c: In function ‘init_misc_binfmt’:
fs/binfmt_misc.c:827:21: error: assignment to ‘struct ctl_table_header *’ from incompatible pointer type ‘struct sysctl_header *’ [-Werror=incompatible-pointer-types]
  827 |  binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");

Fixes: ee9efac48a08("sysctl: add helper to register a sysctl mount point")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Tong Zhang <ztong0001@gmail.com>
---
 include/linux/sysctl.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index 180adf7da785..6353d6db69b2 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -265,7 +265,7 @@ static inline struct ctl_table_header *register_sysctl_table(struct ctl_table *
 	return NULL;
 }
 
-static inline struct sysctl_header *register_sysctl_mount_point(const char *path)
+static inline struct ctl_table_header *register_sysctl_mount_point(const char *path)
 {
 	return NULL;
 }
-- 
2.25.1

Re: [PATCH v1] binfmt_misc: fix crash when load/unload module

[Tong Zhang]

On Mon, Jan 24, 2022 at 2:40 AM Christian Brauner <brauner@kernel.org> wrote:
> The fix itself is obviously needed.
>
> However, afaict the previous patch introduced another bug and this patch
> right here doesn't fix it either.
>
> Namely, if you set CONFIG_SYSCTL=n and CONFIG_BINFMT_MISC={y,m}, then
> register_sysctl_mount_point() will return NULL causing modprobe
> binfmt_misc to fail. However, before 3ba442d5331f ("fs: move binfmt_misc
> sysctl to its own file") loading binfmt_misc would've succeeded even if
> fs/binfmt_misc wasn't created in kernel/sysctl.c. Afaict, that goes for
> both CONFIG_SYSCTL={y,n} since even in the CONFIG_SYSCTL=y case the
> kernel would've moved on if creating the sysctl header would've failed.
> And that makes sense since binfmt_misc is mountable wherever, not just
> at fs/binfmt_misc.
>
> All that indicates that the correct fix here would be to simply:
>
> binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");
>
> without checking for an error. That should fully restore the old
> behavior.
>

Thanks! That makes sense.
I modified the patch according to your comment, added another fix for
the return type issue and sent a v2.
Thanks again.
- Tong

Re: [PATCH v1] binfmt_misc: fix crash when load/unload module

[Andrew Morton]

On Mon, 24 Jan 2022 19:40:53 +0800 kernel test robot <lkp@intel.com> wrote:

> Hi Tong,
> 
> 
> >> fs/binfmt_misc.c:828:21: error: incompatible pointer types assigning to 'struct ctl_table_header *' from 'struct sysctl_header *' [-Werror,-Wincompatible-pointer-types]
>            binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");
>                               ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    1 error generated.
> 
> 
> vim +828 fs/binfmt_misc.c
> 
>    821	
>    822	static int __init init_misc_binfmt(void)
>    823	{
>    824		int err = register_filesystem(&bm_fs_type);
>    825		if (!err)
>    826			insert_binfmt(&misc_format);
>    827	
>  > 828		binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");
>    829		if (!binfmt_misc_header) {
>    830			pr_warn("Failed to create fs/binfmt_misc sysctl mount point");
>    831			return -ENOMEM;
>    832		}
>    833		return 0;
>    834	}
>    835	

This is actually a blooper in Luis's "sysctl: add helper to register a
sysctl mount point".

Please test, review, ridicule, etc:

From: Andrew Morton <akpm@linux-foundation.org>
Subject: include/linux/sysctl.h: fix register_sysctl_mount_point() return type

The CONFIG_SYSCTL=n stub returns the wrong type.

Fixes: ee9efac48a082 ("sysctl: add helper to register a sysctl mount point")
Cc: Luis Chamberlain <mcgrof@kernel.org>
Cc: Tong Zhang <ztong0001@gmail.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
---

 include/linux/sysctl.h |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

--- a/include/linux/sysctl.h~a
+++ a/include/linux/sysctl.h
@@ -265,7 +265,7 @@ static inline struct ctl_table_header *r
 	return NULL;
 }
 
-static inline struct sysctl_header *register_sysctl_mount_point(const char *path)
+static inline struct ctl_table_header *register_sysctl_mount_point(const char *path)
 {
 	return NULL;
 }
_

Re: [PATCH v1] binfmt_misc: fix crash when load/unload module

[Luis Chamberlain]

On Mon, Jan 24, 2022 at 03:16:11PM -0800, Andrew Morton wrote:
> On Mon, 24 Jan 2022 19:40:53 +0800 kernel test robot <lkp@intel.com> wrote:
> 
> > Hi Tong,
> > 
> > 
> > >> fs/binfmt_misc.c:828:21: error: incompatible pointer types assigning to 'struct ctl_table_header *' from 'struct sysctl_header *' [-Werror,-Wincompatible-pointer-types]
> >            binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");
> >                               ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    1 error generated.
> > 
> > 
> > vim +828 fs/binfmt_misc.c
> > 
> >    821	
> >    822	static int __init init_misc_binfmt(void)
> >    823	{
> >    824		int err = register_filesystem(&bm_fs_type);
> >    825		if (!err)
> >    826			insert_binfmt(&misc_format);
> >    827	
> >  > 828		binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");
> >    829		if (!binfmt_misc_header) {
> >    830			pr_warn("Failed to create fs/binfmt_misc sysctl mount point");
> >    831			return -ENOMEM;
> >    832		}
> >    833		return 0;
> >    834	}
> >    835	
> 
> This is actually a blooper in Luis's "sysctl: add helper to register a
> sysctl mount point".
> 
> Please test, review, ridicule, etc:
> 
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: include/linux/sysctl.h: fix register_sysctl_mount_point() return type
> 
> The CONFIG_SYSCTL=n stub returns the wrong type.
> 
> Fixes: ee9efac48a082 ("sysctl: add helper to register a sysctl mount point")
> Cc: Luis Chamberlain <mcgrof@kernel.org>
> Cc: Tong Zhang <ztong0001@gmail.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

Acked-by: Luis Chamberlain <mcgrof@kernel.org>

  Luis

Re: [PATCH v2 1/2] binfmt_misc: fix crash when load/unload module

[Luis Chamberlain]

On Mon, Jan 24, 2022 at 10:18:12AM -0800, Tong Zhang wrote:
> We should unregister the table upon module unload otherwise something
> horrible will happen when we load binfmt_misc module again. Also note
> that we should keep value returned by register_sysctl_mount_point() and
> release it later, otherwise it will leak.
> Also, per Christian's comment, to fully restore the old behavior that
> won't break userspace the check(binfmt_misc_header) should be
> eliminated.
> 
> reproduce:
> modprobe binfmt_misc
> modprobe -r binfmt_misc
> modprobe binfmt_misc
> modprobe -r binfmt_misc
> modprobe binfmt_misc
> 
> [   18.032038] Call Trace:
> [   18.032108]  <TASK>
> [   18.032169]  dump_stack_lvl+0x34/0x44
> [   18.032273]  __register_sysctl_table+0x6f4/0x720
> [   18.032397]  ? preempt_count_sub+0xf/0xb0
> [   18.032508]  ? 0xffffffffc0040000
> [   18.032600]  init_misc_binfmt+0x2d/0x1000 [binfmt_misc]
> [   18.042520] binfmt_misc: Failed to create fs/binfmt_misc sysctl mount point
> modprobe: can't load module binfmt_misc (kernel/fs/binfmt_misc.ko): Cannot allocate memory
> [   18.063549] binfmt_misc: Failed to create fs/binfmt_misc sysctl mount point
> [   18.204779] BUG: unable to handle page fault for address: fffffbfff8004802
> 
> Fixes: 3ba442d5331f ("fs: move binfmt_misc sysctl to its own file")
> Co-developed-by: Christian Brauner<brauner@kernel.org>
> Signed-off-by: Tong Zhang <ztong0001@gmail.com>

Acked-by: Luis Chamberlain <mcgrof@kernel.org>

  Luis

Re: [PATCH v1] binfmt_misc: fix crash when load/unload module

[Murphy Zhou]

On Tue, Jan 25, 2022 at 4:53 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Mon, 24 Jan 2022 19:40:53 +0800 kernel test robot <lkp@intel.com> wrote:
>
> > Hi Tong,
> >
> >
> > >> fs/binfmt_misc.c:828:21: error: incompatible pointer types assigning to 'struct ctl_table_header *' from 'struct sysctl_header *' [-Werror,-Wincompatible-pointer-types]
> >            binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");
> >                               ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> >    1 error generated.
> >
> >
> > vim +828 fs/binfmt_misc.c
> >
> >    821
> >    822        static int __init init_misc_binfmt(void)
> >    823        {
> >    824                int err = register_filesystem(&bm_fs_type);
> >    825                if (!err)
> >    826                        insert_binfmt(&misc_format);
> >    827
> >  > 828                binfmt_misc_header = register_sysctl_mount_point("fs/binfmt_misc");
> >    829                if (!binfmt_misc_header) {
> >    830                        pr_warn("Failed to create fs/binfmt_misc sysctl mount point");
> >    831                        return -ENOMEM;
> >    832                }
> >    833                return 0;
> >    834        }
> >    835
>
> This is actually a blooper in Luis's "sysctl: add helper to register a
> sysctl mount point".
>
> Please test, review, ridicule, etc:
>
> From: Andrew Morton <akpm@linux-foundation.org>
> Subject: include/linux/sysctl.h: fix register_sysctl_mount_point() return type
>
> The CONFIG_SYSCTL=n stub returns the wrong type.
>
> Fixes: ee9efac48a082 ("sysctl: add helper to register a sysctl mount point")
> Cc: Luis Chamberlain <mcgrof@kernel.org>
> Cc: Tong Zhang <ztong0001@gmail.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
>  include/linux/sysctl.h |    2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> --- a/include/linux/sysctl.h~a
> +++ a/include/linux/sysctl.h
> @@ -265,7 +265,7 @@ static inline struct ctl_table_header *r
>         return NULL;
>  }
>
> -static inline struct sysctl_header *register_sysctl_mount_point(const char *path)
> +static inline struct ctl_table_header *register_sysctl_mount_point(const char *path)
>  {
>         return NULL;
>  }
> _

Still panic with this patch on Linux-next tree:

[ 1128.275515] LTP: starting binfmt_misc02 (binfmt_misc02.sh)
[ 1128.303975] CPU: 1 PID: 107182 Comm: modprobe Kdump: loaded
Tainted: G        W         5.17.0-rc1-next-20220125+ #1
[ 1128.305264] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
[ 1128.305992] Call Trace:
[ 1128.306376]  <TASK>
[ 1128.306682]  dump_stack_lvl+0x34/0x44
[ 1128.307211]  __register_sysctl_table+0x2c7/0x4a0
[ 1128.307846]  ? load_module+0xb37/0xbb0
[ 1128.308339]  ? 0xffffffffc01b6000
[ 1128.308762]  init_misc_binfmt+0x32/0x1000 [binfmt_misc]
[ 1128.309402]  do_one_initcall+0x44/0x200
[ 1128.309937]  ? kmem_cache_alloc_trace+0x163/0x2c0
[ 1128.310535]  do_init_module+0x5c/0x260
[ 1128.311045]  __do_sys_finit_module+0xb4/0x120
[ 1128.311603]  do_syscall_64+0x3b/0x90
[ 1128.312088]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 1128.312755] RIP: 0033:0x7f929ab85fbd
[ 1128.313204] Code: 5b 41 5c c3 66 0f 1f 84 00 00 00 00 00 f3 0f 1e
fa 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24
08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 3b ee 0e 00 f7 d8 64 89
01 48
[ 1128.315402] RSP: 002b:00007ffe5d30ef48 EFLAGS: 00000246 ORIG_RAX:
0000000000000139
[ 1128.316312] RAX: ffffffffffffffda RBX: 00007f929bd2fc60 RCX: 00007f929ab85fbd
[ 1128.317170] RDX: 0000000000000000 RSI: 00007f929b264962 RDI: 0000000000000003
[ 1128.318032] RBP: 0000000000040000 R08: 0000000000000000 R09: 00007ffe5d30f080
[ 1128.318895] R10: 0000000000000003 R11: 0000000000000246 R12: 00007f929b264962
[ 1128.319768] R13: 00007f929bd2fd70 R14: 00007f929bd2fc60 R15: 00007f929bd2ff30
[ 1128.320642]  </TASK>
[ 1128.320948] binfmt_misc: Failed to create fs/binfmt_misc sysctl mount point
[ 1128.338732] BUG: unable to handle page fault for address: ffffffffc089d010
[ 1128.340439] #PF: supervisor read access in kernel mode
[ 1128.341072] #PF: error_code(0x0000) - not-present page
[ 1128.341702] PGD ea15067 P4D ea15067 PUD ea17067 PMD 1021e4067 PTE 0
[ 1128.342481] Oops: 0000 [#1] PREEMPT SMP PTI
[ 1128.343003] CPU: 1 PID: 107183 Comm: binfmt_misc02.s Kdump: loaded
Tainted: G        W         5.17.0-rc1-next-20220125+ #1
[ 1128.344326] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
[ 1128.345033] RIP: 0010:search_binary_handler+0xb0/0x250
[ 1128.345678] Code: 85 c0 0f 85 62 01 00 00 48 c7 c7 48 35 ef 87 41
bc fe ff ff ff e8 a0 d8 77 00 48 8b 1d 79 fa 6d 01 48 81 fb 00 06 47
87 74 5d <48> 8b 7b 10 e8 77 11 e1 ff 84 c0 74 44 48 c7 c7 48 35 ef 87
e8 a7
[ 1128.347879] RSP: 0018:ffffb72900813e48 EFLAGS: 00010206
[ 1128.348575] RAX: 0000000000000000 RBX: ffffffffc089d000 RCX: 0000000000000000
[ 1128.349468] RDX: 0000000000000000 RSI: ffff8f67921d9cc0 RDI: ffffffff87ef3548
[ 1128.350334] RBP: ffff8f678d18ec00 R08: 0000000000000000 R09: 0000000000000001
[ 1128.351201] R10: 0000000000000000 R11: ffff8f6792129f10 R12: 00000000fffffffe
[ 1128.352064] R13: 000000000001a2af R14: 0000000000000001 R15: ffff8f67919f8000
[ 1128.352927] FS:  00007f7f21d4a740(0000) GS:ffff8f67bbd00000(0000)
knlGS:0000000000000000
[ 1128.353903] CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 1128.354608] CR2: ffffffffc089d010 CR3: 000000010df8a004 CR4: 00000000007706e0
[ 1128.355469] DR0: 0000000000000000 DR1: 0000000000000000 DR2: 0000000000000000
[ 1128.356335] DR3: 0000000000000000 DR6: 00000000fffe0ff0 DR7: 0000000000000400
[ 1128.357196] PKRU: 55555554
[ 1128.357542] Call Trace:
[ 1128.357900]  <TASK>
[ 1128.358182]  exec_binprm+0x51/0x1a0
[ 1128.358626]  bprm_execve.part.0+0x16c/0x210
[ 1128.359142]  do_execveat_common.isra.0+0x156/0x1c0
[ 1128.359736]  __x64_sys_execve+0x33/0x40
[ 1128.360213]  do_syscall_64+0x3b/0x90
[ 1128.360668]  entry_SYSCALL_64_after_hwframe+0x44/0xae
[ 1128.361287] RIP: 0033:0x7f7f21e2aabb
[ 1128.361739] Code: fb fe ff ff 48 8d 3d 24 4b 12 00 e8 3f cd fa ff
e9 ea fe ff ff 66 2e 0f 1f 84 00 00 00 00 00 f3 0f 1e fa b8 3b 00 00
00 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 8b 0d 3d c3 11 00 f7 d8 64 89
01 48
[ 1128.363940] RSP: 002b:00007ffec98f2f18 EFLAGS: 00000246 ORIG_RAX:
000000000000003b
[ 1128.364853] RAX: ffffffffffffffda RBX: 0000000000000000 RCX: 00007f7f21e2aabb
[ 1128.365713] RDX: 00007f7f2347faa0 RSI: 00007f7f233e9940 RDI: 00007f7f2347f110
[ 1128.366576] RBP: 00007f7f2347f110 R08: 00007f7f23445420 R09: 0000000000000000
[ 1128.367433] R10: 0000000000000008 R11: 0000000000000246 R12: 00000000ffffffff
[ 1128.368296] R13: 00007f7f233e9940 R14: 00007f7f2347faa0 R15: 00007f7f2347f360
[ 1128.369160]  </TASK>
[ 1128.369462] Modules linked in: brd overlay exfat vfat fat ext2 loop
rfkill intel_rapl_msr intel_rapl_common isst_if_common nfit joydev
virtio_balloon sunrpc i2c_piix4 pcspkr ext4 mbcache jbd2 drm fuse xfs
libcrc32c ata_generic crct10dif_pclmul ata_piix crc32_pclmul
crc32c_intel virtio_net libata net_failover serio_raw
ghash_clmulni_intel virtio_blk failover [last unloaded: binfmt_misc]
[ 1128.373485] CR2: ffffffffc089d010



Testing patch on Linus tree.

Re: [PATCH v1] binfmt_misc: fix crash when load/unload module

[Tong Zhang]

On Tue, Jan 25, 2022 at 9:04 PM Murphy Zhou <jencce.kernel@gmail.com> wrote:
>
> Still panic with this patch on Linux-next tree:
>
> [ 1128.275515] LTP: starting binfmt_misc02 (binfmt_misc02.sh)
> [ 1128.303975] CPU: 1 PID: 107182 Comm: modprobe Kdump: loaded
> Tainted: G        W         5.17.0-rc1-next-20220125+ #1
> [ 1128.305264] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
> [ 1128.305992] Call Trace:
> [ 1128.306376]  <TASK>
> [ 1128.306682]  dump_stack_lvl+0x34/0x44
> [ 1128.307211]  __register_sysctl_table+0x2c7/0x4a0
> [ 1128.307846]  ? load_module+0xb37/0xbb0
> [ 1128.308339]  ? 0xffffffffc01b6000
> [ 1128.308762]  init_misc_binfmt+0x32/0x1000 [binfmt_misc]
> [ 1128.309402]  do_one_initcall+0x44/0x200
> [ 1128.309937]  ? kmem_cache_alloc_trace+0x163/0x2c0
> [ 1128.310535]  do_init_module+0x5c/0x260
> [ 1128.311045]  __do_sys_finit_module+0xb4/0x120
> [ 1128.311603]  do_syscall_64+0x3b/0x90
> [ 1128.312088]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> [ 1128.312755] RIP: 0033:0x7f929ab85fbd
>
> Testing patch on Linus tree.

Hi Murphy,
Did you apply this patch?
Link: https://lkml.kernel.org/r/20220124181812.1869535-2-ztong0001@gmail.com
I tested it on top of the current master branch and it works on my
setup using the reproducer I mentioned.
Could you share your test script?
Thanks,
- Tong

Re: [PATCH v1] binfmt_misc: fix crash when load/unload module

[Tong Zhang]

On Tue, Jan 25, 2022 at 9:23 PM Tong Zhang <ztong0001@gmail.com> wrote:
>
> On Tue, Jan 25, 2022 at 9:04 PM Murphy Zhou <jencce.kernel@gmail.com> wrote:
> >
> > Still panic with this patch on Linux-next tree:
> >
> > [ 1128.275515] LTP: starting binfmt_misc02 (binfmt_misc02.sh)
> > [ 1128.303975] CPU: 1 PID: 107182 Comm: modprobe Kdump: loaded
> > Tainted: G        W         5.17.0-rc1-next-20220125+ #1
> > [ 1128.305264] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
> > [ 1128.305992] Call Trace:
> > [ 1128.306376]  <TASK>
> > [ 1128.306682]  dump_stack_lvl+0x34/0x44
> > [ 1128.307211]  __register_sysctl_table+0x2c7/0x4a0
> > [ 1128.307846]  ? load_module+0xb37/0xbb0
> > [ 1128.308339]  ? 0xffffffffc01b6000
> > [ 1128.308762]  init_misc_binfmt+0x32/0x1000 [binfmt_misc]
> > [ 1128.309402]  do_one_initcall+0x44/0x200
> > [ 1128.309937]  ? kmem_cache_alloc_trace+0x163/0x2c0
> > [ 1128.310535]  do_init_module+0x5c/0x260
> > [ 1128.311045]  __do_sys_finit_module+0xb4/0x120
> > [ 1128.311603]  do_syscall_64+0x3b/0x90
> > [ 1128.312088]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> > [ 1128.312755] RIP: 0033:0x7f929ab85fbd
> >
> > Testing patch on Linus tree.
>
> Hi Murphy,
> Did you apply this patch?
> Link: https://lkml.kernel.org/r/20220124181812.1869535-2-ztong0001@gmail.com
> I tested it on top of the current master branch and it works on my
> setup using the reproducer I mentioned.
> Could you share your test script?
> Thanks,
> - Tong

I can find binfmt_misc02.sh on github, and running the following
command shows: failed 0.

./runltp -s binfmt_misc
Running tests.......
<<<test_start>>>
tag=binfmt_misc01 stime=1643178454
cmdline="binfmt_misc01.sh"
contacts=""
analysis=exit
<<<test_output>>>
[   90.908282] LTP: starting binfmt_misc01 (binfmt_misc01.sh)
binfmt_misc01 1 TINFO: timeout per run is 0h 5m 0s
binfmt_misc01 1 TPASS: Failed to register a binary type
binfmt_misc01 2 TPASS: Failed to register a binary type
binfmt_misc01 3 TPASS: Failed to register a binary type
binfmt_misc01 4 TPASS: Failed to register a binary type
binfmt_misc01 5 TPASS: Failed to register a binary type
binfmt_misc01 6 TPASS: Failed to register a binary type
binfmt_misc01 7 TPASS: Failed to register a binary type
binfmt_misc01 8 TPASS: Failed to register a binary type
binfmt_misc01 9 TPASS: Failed to register a binary type

Summary:
passed   9
failed   0
broken   0
skipped  0
warnings 0
<<<execution_status>>>
initiation_status="ok"
duration=0 termination_type=exited termination_id=0 corefile=no
cutime=2 cstime=17
<<<test_end>>>
<<<test_start>>>
tag=binfmt_misc02 stime=1643178454
cmdline="binfmt_misc02.sh"
contacts=""
analysis=exit
<<<test_output>>>
[   91.133399] LTP: starting binfmt_misc02 (binfmt_misc02.sh)
incrementing stop
binfmt_misc02 1 TINFO: timeout per run is 0h 5m 0s
binfmt_misc02 1 TPASS: Recognise and unrecognise a binary type as expected
binfmt_misc02 2 TPASS: Recognise and unrecognise a binary type as expected
binfmt_misc02 3 TPASS: Recognise and unrecognise a binary type as expected
binfmt_misc02 4 TPASS: Recognise and unrecognise a binary type as expected
binfmt_misc02 5 TPASS: Fail to recognise a binary type
binfmt_misc02 6 TPASS: Fail to recognise a binary type

Summary:
passed   6
failed   0
broken   0
skipped  0
warnings 0
<<<execution_status>>>
initiation_status="ok"
duration=0 termination_type=exited termination_id=0 corefile=no
cutime=3 cstime=25
<<<test_end>>>
INFO: ltp-pan reported all tests PASS
LTP Version: 20220121-9-g010e4f783

       ###############################################################

            Done executing testcases.
            LTP Version:  20220121-9-g010e4f783
       ###############################################################

Re: [PATCH v1] binfmt_misc: fix crash when load/unload module

[Murphy Zhou]

On Tue, Jan 25, 2022 at 10:33:22PM -0800, Tong Zhang wrote:
> On Tue, Jan 25, 2022 at 9:23 PM Tong Zhang <ztong0001@gmail.com> wrote:
> >
> > On Tue, Jan 25, 2022 at 9:04 PM Murphy Zhou <jencce.kernel@gmail.com> wrote:
> > >
> > > Still panic with this patch on Linux-next tree:
> > >
> > > [ 1128.275515] LTP: starting binfmt_misc02 (binfmt_misc02.sh)
> > > [ 1128.303975] CPU: 1 PID: 107182 Comm: modprobe Kdump: loaded
> > > Tainted: G        W         5.17.0-rc1-next-20220125+ #1
> > > [ 1128.305264] Hardware name: Red Hat KVM, BIOS 0.5.1 01/01/2011
> > > [ 1128.305992] Call Trace:
> > > [ 1128.306376]  <TASK>
> > > [ 1128.306682]  dump_stack_lvl+0x34/0x44
> > > [ 1128.307211]  __register_sysctl_table+0x2c7/0x4a0
> > > [ 1128.307846]  ? load_module+0xb37/0xbb0
> > > [ 1128.308339]  ? 0xffffffffc01b6000
> > > [ 1128.308762]  init_misc_binfmt+0x32/0x1000 [binfmt_misc]
> > > [ 1128.309402]  do_one_initcall+0x44/0x200
> > > [ 1128.309937]  ? kmem_cache_alloc_trace+0x163/0x2c0
> > > [ 1128.310535]  do_init_module+0x5c/0x260
> > > [ 1128.311045]  __do_sys_finit_module+0xb4/0x120
> > > [ 1128.311603]  do_syscall_64+0x3b/0x90
> > > [ 1128.312088]  entry_SYSCALL_64_after_hwframe+0x44/0xae
> > > [ 1128.312755] RIP: 0033:0x7f929ab85fbd
> > >
> > > Testing patch on Linus tree.
> >
> > Hi Murphy,
> > Did you apply this patch?
> > Link: https://lkml.kernel.org/r/20220124181812.1869535-2-ztong0001@gmail.com
> > I tested it on top of the current master branch and it works on my
> > setup using the reproducer I mentioned.
> > Could you share your test script?
> > Thanks,
> > - Tong
> 
> I can find binfmt_misc02.sh on github, and running the following
> command shows: failed 0.
> 
> ./runltp -s binfmt_misc
> Running tests.......
> <<<test_start>>>
> tag=binfmt_misc01 stime=1643178454
> cmdline="binfmt_misc01.sh"
> contacts=""
> analysis=exit
> <<<test_output>>>
> [   90.908282] LTP: starting binfmt_misc01 (binfmt_misc01.sh)
> binfmt_misc01 1 TINFO: timeout per run is 0h 5m 0s
> binfmt_misc01 1 TPASS: Failed to register a binary type
> binfmt_misc01 2 TPASS: Failed to register a binary type
> binfmt_misc01 3 TPASS: Failed to register a binary type
> binfmt_misc01 4 TPASS: Failed to register a binary type
> binfmt_misc01 5 TPASS: Failed to register a binary type
> binfmt_misc01 6 TPASS: Failed to register a binary type
> binfmt_misc01 7 TPASS: Failed to register a binary type
> binfmt_misc01 8 TPASS: Failed to register a binary type
> binfmt_misc01 9 TPASS: Failed to register a binary type
> 
> Summary:
> passed   9
> failed   0
> broken   0
> skipped  0
> warnings 0
> <<<execution_status>>>
> initiation_status="ok"
> duration=0 termination_type=exited termination_id=0 corefile=no
> cutime=2 cstime=17
> <<<test_end>>>
> <<<test_start>>>
> tag=binfmt_misc02 stime=1643178454
> cmdline="binfmt_misc02.sh"
> contacts=""
> analysis=exit
> <<<test_output>>>
> [   91.133399] LTP: starting binfmt_misc02 (binfmt_misc02.sh)
> incrementing stop
> binfmt_misc02 1 TINFO: timeout per run is 0h 5m 0s
> binfmt_misc02 1 TPASS: Recognise and unrecognise a binary type as expected
> binfmt_misc02 2 TPASS: Recognise and unrecognise a binary type as expected
> binfmt_misc02 3 TPASS: Recognise and unrecognise a binary type as expected
> binfmt_misc02 4 TPASS: Recognise and unrecognise a binary type as expected
> binfmt_misc02 5 TPASS: Fail to recognise a binary type
> binfmt_misc02 6 TPASS: Fail to recognise a binary type
> 
> Summary:
> passed   6
> failed   0
> broken   0
> skipped  0
> warnings 0
> <<<execution_status>>>
> initiation_status="ok"
> duration=0 termination_type=exited termination_id=0 corefile=no
> cutime=3 cstime=25
> <<<test_end>>>
> INFO: ltp-pan reported all tests PASS
> LTP Version: 20220121-9-g010e4f783
> 
>        ###############################################################
> 
>             Done executing testcases.
>             LTP Version:  20220121-9-g010e4f783
>        ###############################################################

Ya, looks like it's working. No panic on next-20220128 tree.

Thanks,
-- 
Murphy