2023-03-15 03:45:31

by Muchun Song

Subject: [PATCH] mm: kfence: fix using kfence_metadata without initialization in show_object()

The variable kfence_metadata is initialized in kfence_init_pool(), then, it is
not initialized if kfence is disabled after booting. In this case, kfence_metadata
will be used (e.g. ->lock and ->state fields) without initialization when reading
/sys/kernel/debug/kfence/objects. There will be a warning if you enable
CONFIG_DEBUG_SPINLOCK. Fix it by creating debugfs files when necessary.

Fixes: 0ce20dd84089 ("mm: add Kernel Electric-Fence infrastructure")
Signed-off-by: Muchun Song <[email protected]>
---
mm/kfence/core.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/mm/kfence/core.c b/mm/kfence/core.c
index 5349c37a5dac..79c94ee55f97 100644
--- a/mm/kfence/core.c
+++ b/mm/kfence/core.c
@@ -726,10 +726,14 @@ static const struct seq_operations objects_sops = {
};
DEFINE_SEQ_ATTRIBUTE(objects);

-static int __init kfence_debugfs_init(void)
+static int kfence_debugfs_init(void)
{
- struct dentry *kfence_dir = debugfs_create_dir("kfence", NULL);
+ struct dentry *kfence_dir;

+ if (!READ_ONCE(kfence_enabled))
+ return 0;
+
+ kfence_dir = debugfs_create_dir("kfence", NULL);
debugfs_create_file("stats", 0444, kfence_dir, NULL, &stats_fops);
debugfs_create_file("objects", 0400, kfence_dir, NULL, &objects_fops);
return 0;
@@ -883,6 +887,8 @@ static int kfence_init_late(void)
}

kfence_init_enable();
+ kfence_debugfs_init();
+
return 0;
}

--
2.11.0

2023-03-15 08:08:27

by Marco Elver

Subject: Re: [PATCH] mm: kfence: fix using kfence_metadata without initialization in show_object()

On Wed, 15 Mar 2023 at 04:45, Muchun Song <[email protected]> wrote:
>
> The variable kfence_metadata is initialized in kfence_init_pool(), then, it is
> not initialized if kfence is disabled after booting. In this case, kfence_metadata
> will be used (e.g. ->lock and ->state fields) without initialization when reading
> /sys/kernel/debug/kfence/objects. There will be a warning if you enable
> CONFIG_DEBUG_SPINLOCK. Fix it by creating debugfs files when necessary.
>
> Fixes: 0ce20dd84089 ("mm: add Kernel Electric-Fence infrastructure")
> Signed-off-by: Muchun Song <[email protected]>

Tested-by: Marco Elver <[email protected]>
Reviewed-by: Marco Elver <[email protected]>

Good catch!

> ---
> mm/kfence/core.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/mm/kfence/core.c b/mm/kfence/core.c
> index 5349c37a5dac..79c94ee55f97 100644
> --- a/mm/kfence/core.c
> +++ b/mm/kfence/core.c
> @@ -726,10 +726,14 @@ static const struct seq_operations objects_sops = {
> };
> DEFINE_SEQ_ATTRIBUTE(objects);
>
> -static int __init kfence_debugfs_init(void)
> +static int kfence_debugfs_init(void)
> {
> - struct dentry *kfence_dir = debugfs_create_dir("kfence", NULL);
> + struct dentry *kfence_dir;
>
> + if (!READ_ONCE(kfence_enabled))
> + return 0;
> +
> + kfence_dir = debugfs_create_dir("kfence", NULL);
> debugfs_create_file("stats", 0444, kfence_dir, NULL, &stats_fops);
> debugfs_create_file("objects", 0400, kfence_dir, NULL, &objects_fops);
> return 0;
> @@ -883,6 +887,8 @@ static int kfence_init_late(void)
> }
>
> kfence_init_enable();
> + kfence_debugfs_init();
> +
> return 0;
> }
>
> --
> 2.11.0
>

2023-03-15 19:54:42

by Andrew Morton

Subject: Re: [PATCH] mm: kfence: fix using kfence_metadata without initialization in show_object()

On Wed, 15 Mar 2023 09:07:40 +0100 Marco Elver <[email protected]> wrote:

> On Wed, 15 Mar 2023 at 04:45, Muchun Song <[email protected]> wrote:
> >
> > The variable kfence_metadata is initialized in kfence_init_pool(), then, it is
> > not initialized if kfence is disabled after booting. In this case, kfence_metadata
> > will be used (e.g. ->lock and ->state fields) without initialization when reading
> > /sys/kernel/debug/kfence/objects. There will be a warning if you enable
> > CONFIG_DEBUG_SPINLOCK. Fix it by creating debugfs files when necessary.
> >
> > Fixes: 0ce20dd84089 ("mm: add Kernel Electric-Fence infrastructure")
> > Signed-off-by: Muchun Song <[email protected]>
>
> Tested-by: Marco Elver <[email protected]>
> Reviewed-by: Marco Elver <[email protected]>

Thanks, I'll add cc:stable to this.

I assume the warning is the only known adverse effect of this bug?

2023-03-15 21:05:57

by Marco Elver

Subject: Re: [PATCH] mm: kfence: fix using kfence_metadata without initialization in show_object()

On Wed, 15 Mar 2023 at 20:54, Andrew Morton <[email protected]> wrote:
>
> On Wed, 15 Mar 2023 09:07:40 +0100 Marco Elver <[email protected]> wrote:
>
> > On Wed, 15 Mar 2023 at 04:45, Muchun Song <[email protected]> wrote:
> > >
> > > The variable kfence_metadata is initialized in kfence_init_pool(), then, it is
> > > not initialized if kfence is disabled after booting. In this case, kfence_metadata
> > > will be used (e.g. ->lock and ->state fields) without initialization when reading
> > > /sys/kernel/debug/kfence/objects. There will be a warning if you enable
> > > CONFIG_DEBUG_SPINLOCK. Fix it by creating debugfs files when necessary.
> > >
> > > Fixes: 0ce20dd84089 ("mm: add Kernel Electric-Fence infrastructure")
> > > Signed-off-by: Muchun Song <[email protected]>
> >
> > Tested-by: Marco Elver <[email protected]>
> > Reviewed-by: Marco Elver <[email protected]>
>
> Thanks, I'll add cc:stable to this.
>
> I assume the warning is the only known adverse effect of this bug?

For architectures where the initial spinlock state is 0, the warning
is the only issue. For architectures where that's not the case, it
might result in lockup of the task querying the 'objects' file --
which isn't the case for any arch that supports KFENCE by the looks of
it (last I checked 'sh' and 'parisc' don't support KFENCE).

Thanks,
-- Marco