[PATCH] mm: export copy_to_kernel_nofault

Arnd Bergmann posted 1 patch 1 month, 1 week ago
mm/maccess.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] mm: export copy_to_kernel_nofault
Posted by Arnd Bergmann 1 month, 1 week ago
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
Posted by Christoph Hellwig 1 month ago
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
Posted by Marco Elver 1 month ago
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
Posted by Christoph Hellwig 1 month ago
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
Posted by Marco Elver 1 month ago
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
Posted by Christoph Hellwig 1 month ago
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
Posted by Andrew Morton 2 weeks, 4 days ago
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
Posted by Andrey Konovalov 1 month, 1 week ago
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
Posted by David Hildenbrand 1 month, 1 week ago
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