2022-01-24 15:04:17

by Tong Zhang

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

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 <[email protected]>
---
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


2022-01-24 19:04:12

by Christian Brauner

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

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 <[email protected]>
> ---
> 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
>

2022-01-24 19:06:42

by kernel test robot

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

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/[email protected]/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 <[email protected]>

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/[email protected]

2022-01-24 19:11:29

by kernel test robot

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

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/[email protected]/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 <[email protected]>

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/[email protected]

2022-01-24 19:52:03

by Tong Zhang

[permalink] [raw]
Subject: [PATCH v2 0/2] Fix regression on binfmt_misc

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

2022-01-24 19:52:07

by Tong Zhang

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

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<[email protected]>
Signed-off-by: Tong Zhang <[email protected]>
---
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

2022-01-24 19:52:10

by Tong Zhang

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

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 <[email protected]>
Signed-off-by: Tong Zhang <[email protected]>
---
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

2022-01-24 19:52:25

by Tong Zhang

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

On Mon, Jan 24, 2022 at 2:40 AM Christian Brauner <[email protected]> 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

2022-01-25 08:59:01

by Andrew Morton

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

On Mon, 24 Jan 2022 19:40:53 +0800 kernel test robot <[email protected]> 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 <[email protected]>
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 <[email protected]>
Cc: Tong Zhang <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
---

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;
}
_

2022-01-26 03:19:49

by Luis Chamberlain

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

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 <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> Cc: Tong Zhang <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>

Acked-by: Luis Chamberlain <[email protected]>

Luis

2022-01-26 03:23:24

by Luis Chamberlain

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

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<[email protected]>
> Signed-off-by: Tong Zhang <[email protected]>

Acked-by: Luis Chamberlain <[email protected]>

Luis

2022-01-26 18:37:15

by Murphy Zhou

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

On Tue, Jan 25, 2022 at 4:53 PM Andrew Morton <[email protected]> wrote:
>
> On Mon, 24 Jan 2022 19:40:53 +0800 kernel test robot <[email protected]> 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 <[email protected]>
> 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 <[email protected]>
> Cc: Tong Zhang <[email protected]>
> Signed-off-by: Andrew Morton <[email protected]>
> ---
>
> 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.

2022-01-26 18:40:29

by Tong Zhang

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

On Tue, Jan 25, 2022 at 9:04 PM Murphy Zhou <[email protected]> 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/[email protected]
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

2022-01-26 20:07:14

by Tong Zhang

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

On Tue, Jan 25, 2022 at 9:23 PM Tong Zhang <[email protected]> wrote:
>
> On Tue, Jan 25, 2022 at 9:04 PM Murphy Zhou <[email protected]> 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/[email protected]
> 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
###############################################################

2022-01-31 11:52:35

by Murphy Zhou

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

On Tue, Jan 25, 2022 at 10:33:22PM -0800, Tong Zhang wrote:
> On Tue, Jan 25, 2022 at 9:23 PM Tong Zhang <[email protected]> wrote:
> >
> > On Tue, Jan 25, 2022 at 9:04 PM Murphy Zhou <[email protected]> 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/[email protected]
> > 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