[PATCH] mm: export copy_to_kernel_nofault

[PATCH] mm: export copy_to_kernel_nofault

[Arnd Bergmann]

From: Arnd Bergmann <arnd@arndb.de>

This symbol is now used on the kasan test module, so it needs to be
exported.

ERROR: modpost: "copy_to_kernel_nofault" [mm/kasan/kasan_test.ko] undefined!

Fixes: 44749130ffb4 ("kasan: migrate copy_user_test to kunit")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 mm/maccess.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/mm/maccess.c b/mm/maccess.c
index 3ca55ec63a6a..8f0906180a94 100644
--- a/mm/maccess.c
+++ b/mm/maccess.c
@@ -82,6 +82,7 @@ long copy_to_kernel_nofault(void *dst, const void *src, size_t size)
 	pagefault_enable();
 	return -EFAULT;
 }
+EXPORT_SYMBOL_GPL(copy_to_kernel_nofault);
 
 long strncpy_from_kernel_nofault(char *dst, const void *unsafe_addr, long count)
 {
-- 
2.39.5

Re: [PATCH] mm: export copy_to_kernel_nofault

[David Hildenbrand]

On 18.10.24 17:11, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> This symbol is now used on the kasan test module, so it needs to be
> exported.
> 
> ERROR: modpost: "copy_to_kernel_nofault" [mm/kasan/kasan_test.ko] undefined!
> 
> Fixes: 44749130ffb4 ("kasan: migrate copy_user_test to kunit")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>   mm/maccess.c | 1 +
>   1 file changed, 1 insertion(+)
> 
> diff --git a/mm/maccess.c b/mm/maccess.c
> index 3ca55ec63a6a..8f0906180a94 100644
> --- a/mm/maccess.c
> +++ b/mm/maccess.c
> @@ -82,6 +82,7 @@ long copy_to_kernel_nofault(void *dst, const void *src, size_t size)
>   	pagefault_enable();
>   	return -EFAULT;
>   }
> +EXPORT_SYMBOL_GPL(copy_to_kernel_nofault);
>   
>   long strncpy_from_kernel_nofault(char *dst, const void *unsafe_addr, long count)
>   {

Acked-by: David Hildenbrand <david@redhat.com>

-- 
Cheers,

David / dhildenb

Re: [PATCH] mm: export copy_to_kernel_nofault

[Andrey Konovalov]

On Fri, Oct 18, 2024 at 5:11 PM Arnd Bergmann <arnd@kernel.org> wrote:
>
> From: Arnd Bergmann <arnd@arndb.de>
>
> This symbol is now used on the kasan test module, so it needs to be
> exported.
>
> ERROR: modpost: "copy_to_kernel_nofault" [mm/kasan/kasan_test.ko] undefined!
>
> Fixes: 44749130ffb4 ("kasan: migrate copy_user_test to kunit")
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>
> ---
>  mm/maccess.c | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/mm/maccess.c b/mm/maccess.c
> index 3ca55ec63a6a..8f0906180a94 100644
> --- a/mm/maccess.c
> +++ b/mm/maccess.c
> @@ -82,6 +82,7 @@ long copy_to_kernel_nofault(void *dst, const void *src, size_t size)
>         pagefault_enable();
>         return -EFAULT;
>  }
> +EXPORT_SYMBOL_GPL(copy_to_kernel_nofault);
>
>  long strncpy_from_kernel_nofault(char *dst, const void *unsafe_addr, long count)
>  {
> --
> 2.39.5
>

Reviewed-by: Andrey Konovalov <andreyknvl@gmail.com>

Thank you!

Re: [PATCH] mm: export copy_to_kernel_nofault

[Christoph Hellwig]

On Fri, Oct 18, 2024 at 03:11:09PM +0000, Arnd Bergmann wrote:
> From: Arnd Bergmann <arnd@arndb.de>
> 
> This symbol is now used on the kasan test module, so it needs to be
> exported.
> 
> ERROR: modpost: "copy_to_kernel_nofault" [mm/kasan/kasan_test.ko] undefined!

Meh, it would be great not to export internal helpers just because
someone wants to test them.  Please just mark that test built-in only
instead.

Re: [PATCH] mm: export copy_to_kernel_nofault

[Marco Elver]

On Wed, 23 Oct 2024 at 08:53, Christoph Hellwig <hch@infradead.org> wrote:
>
> On Fri, Oct 18, 2024 at 03:11:09PM +0000, Arnd Bergmann wrote:
> > From: Arnd Bergmann <arnd@arndb.de>
> >
> > This symbol is now used on the kasan test module, so it needs to be
> > exported.
> >
> > ERROR: modpost: "copy_to_kernel_nofault" [mm/kasan/kasan_test.ko] undefined!
>
> Meh, it would be great not to export internal helpers just because
> someone wants to test them.  Please just mark that test built-in only
> instead.

We have EXPORT_SYMBOL_IF_KUNIT. See include/kunit/visibility.h -
that's more appropriate, and also adjust kasan_test.c to do
MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING).

Re: [PATCH] mm: export copy_to_kernel_nofault

[Christoph Hellwig]

On Wed, Oct 23, 2024 at 08:57:02AM +0200, Marco Elver wrote:
> On Wed, 23 Oct 2024 at 08:53, Christoph Hellwig <hch@infradead.org> wrote:
> >
> > On Fri, Oct 18, 2024 at 03:11:09PM +0000, Arnd Bergmann wrote:
> > > From: Arnd Bergmann <arnd@arndb.de>
> > >
> > > This symbol is now used on the kasan test module, so it needs to be
> > > exported.
> > >
> > > ERROR: modpost: "copy_to_kernel_nofault" [mm/kasan/kasan_test.ko] undefined!
> >
> > Meh, it would be great not to export internal helpers just because
> > someone wants to test them.  Please just mark that test built-in only
> > instead.
> 
> We have EXPORT_SYMBOL_IF_KUNIT. See include/kunit/visibility.h -
> that's more appropriate, and also adjust kasan_test.c to do
> MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING).

Thats a little better, but at least in this case I still think it is
a very bad idea.  copy_to_kernel_nofault is a perfect vector for
exploit code to probe writing to kernel address without causing faults
so it really should never ever be exported.

Re: [PATCH] mm: export copy_to_kernel_nofault

[Marco Elver]

On Wed, 23 Oct 2024 at 08:59, Christoph Hellwig <hch@infradead.org> wrote:
>
> On Wed, Oct 23, 2024 at 08:57:02AM +0200, Marco Elver wrote:
> > On Wed, 23 Oct 2024 at 08:53, Christoph Hellwig <hch@infradead.org> wrote:
> > >
> > > On Fri, Oct 18, 2024 at 03:11:09PM +0000, Arnd Bergmann wrote:
> > > > From: Arnd Bergmann <arnd@arndb.de>
> > > >
> > > > This symbol is now used on the kasan test module, so it needs to be
> > > > exported.
> > > >
> > > > ERROR: modpost: "copy_to_kernel_nofault" [mm/kasan/kasan_test.ko] undefined!
> > >
> > > Meh, it would be great not to export internal helpers just because
> > > someone wants to test them.  Please just mark that test built-in only
> > > instead.
> >
> > We have EXPORT_SYMBOL_IF_KUNIT. See include/kunit/visibility.h -
> > that's more appropriate, and also adjust kasan_test.c to do
> > MODULE_IMPORT_NS(EXPORTED_FOR_KUNIT_TESTING).
>
> Thats a little better, but at least in this case I still think it is
> a very bad idea.  copy_to_kernel_nofault is a perfect vector for
> exploit code to probe writing to kernel address without causing faults
> so it really should never ever be exported.

Another alternative is to just #ifndef MODULE the offending test case,
so it's only available if built-in. No need to just make the whole
test built-in only. I know there are users of this particular test
that rely on it being a module.

Re: [PATCH] mm: export copy_to_kernel_nofault

[Christoph Hellwig]

On Wed, Oct 23, 2024 at 09:02:23AM +0200, Marco Elver wrote:
> Another alternative is to just #ifndef MODULE the offending test case,
> so it's only available if built-in. No need to just make the whole
> test built-in only. I know there are users of this particular test
> that rely on it being a module.

That sounds good to me.

Re: [PATCH] mm: export copy_to_kernel_nofault

[Andrew Morton]

On Wed, 23 Oct 2024 00:05:36 -0700 Christoph Hellwig <hch@infradead.org> wrote:

> On Wed, Oct 23, 2024 at 09:02:23AM +0200, Marco Elver wrote:
> > Another alternative is to just #ifndef MODULE the offending test case,
> > so it's only available if built-in. No need to just make the whole
> > test built-in only. I know there are users of this particular test
> > that rely on it being a module.
> 
> That sounds good to me.

We still don't have patch which does this, so this series is stalled.

Sabyrzhan, could you please consider this?

Re: [PATCH] mm: export copy_to_kernel_nofault

[Sabyrzhan Tasbolatov]

On Fri, Nov 8, 2024 at 3:29 AM Andrew Morton <akpm@linux-foundation.org> wrote:
>
> On Wed, 23 Oct 2024 00:05:36 -0700 Christoph Hellwig <hch@infradead.org> wrote:
>
> > On Wed, Oct 23, 2024 at 09:02:23AM +0200, Marco Elver wrote:
> > > Another alternative is to just #ifndef MODULE the offending test case,
> > > so it's only available if built-in. No need to just make the whole
> > > test built-in only. I know there are users of this particular test
> > > that rely on it being a module.
> >
> > That sounds good to me.
>
> We still don't have patch which does this, so this series is stalled.
>
> Sabyrzhan, could you please consider this?

I've just posted a follow-up that removes the exported symbol.
See Message-Id: <20250622051906.67374-1-snovitoll@gmail.com>
Sorry for taking so long.