Openwall GNU/*/Linux - a small security-enhanced Linux distro for servers
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date: Tue, 21 Nov 2017 17:29:29 -0800
From: Kees Cook <keescook@...omium.org>
To: Matthew Garrett <mjg59@...gle.com>
Cc: kernel-hardening@...ts.openwall.com
Subject: Re: [PATCH] usercopy: Allow runtime disabling of usercopy fallback

On Tue, Nov 21, 2017 at 4:54 PM, Matthew Garrett <mjg59@...gle.com> wrote:
> Distributions are likely to build with
> CONFIG_HARDENED_USERCOPY_FALLBACK, but individual sites may wish to
> disable that functionality. Add a sysctl that allows the fallback to be
> disabled and then forbids any attempt to re-enable it.

Is it likely that distros will select FALLBACK? I figured by the time
a kernel with this feature got to the distros it'd already gotten a
fair bit of testing, so they'd just leave FALLBACK disabled.

I'll consider this, though I have to refactor the series a bit to move
this patch earlier, etc.

Thanks for looking at it!

-Kees

>
> Signed-off-by: Matthew Garrett <mjg59@...gle.com>
> ---
>  Documentation/sysctl/vm.txt |  9 +++++++++
>  include/linux/mm.h          |  7 +++++++
>  kernel/sysctl.c             | 11 +++++++++++
>  mm/memory.c                 | 17 +++++++++++++++++
>  mm/slab.c                   |  4 +---
>  mm/slub.c                   |  4 +---
>  6 files changed, 46 insertions(+), 6 deletions(-)
>
> diff --git a/Documentation/sysctl/vm.txt b/Documentation/sysctl/vm.txt
> index 9baf66a9ef4e..b494b292fd93 100644
> --- a/Documentation/sysctl/vm.txt
> +++ b/Documentation/sysctl/vm.txt
> @@ -60,6 +60,7 @@ Currently, these files are in /proc/sys/vm:
>  - stat_refresh
>  - swappiness
>  - user_reserve_kbytes
> +- usercopy_fallback
>  - vfs_cache_pressure
>  - watermark_scale_factor
>  - zone_reclaim_mode
> @@ -822,6 +823,14 @@ Changing this takes effect whenever an application requests memory.
>
>  ==============================================================
>
> +usercopy_fallback
> +
> +If 1, attempts to copy data from userland will generate a kernel warning
> +if the target has not been whitelisted, but will still be permitted. If 0,
> +this will result in a kernel BUG(). This may be set from 1 to 0 at runtime,
> +but may not be modified if 0.
> +
> +==============================================================
>  vfs_cache_pressure
>  ------------------
>
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index f8c10d336e42..511237addbf9 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2456,6 +2456,8 @@ int drop_caches_sysctl_handler(struct ctl_table *, int,
>  void drop_slab(void);
>  void drop_slab_node(int nid);
>
> +extern bool usercopy_fallback;
> +
>  #ifndef CONFIG_MMU
>  #define randomize_va_space 0
>  #else
> @@ -2602,5 +2604,10 @@ void __init setup_nr_node_ids(void);
>  static inline void setup_nr_node_ids(void) {}
>  #endif
>
> +#ifdef CONFIG_HARDENED_USERCOPY
> +int usercopy_fallback_handler(struct ctl_table *table, int write,
> +                           void __user *buffer, size_t *length, loff_t *ppos);
> +#endif
> +
>  #endif /* __KERNEL__ */
>  #endif /* _LINUX_MM_H */
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 423554ad3610..ba3366335b40 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1663,6 +1663,17 @@ static struct ctl_table vm_table[] = {
>                 .extra1         = (void *)&mmap_rnd_compat_bits_min,
>                 .extra2         = (void *)&mmap_rnd_compat_bits_max,
>         },
> +#endif
> +#ifdef CONFIG_HARDENED_USERCOPY
> +       {
> +               .procname       = "usercopy_fallback",
> +               .data           = &usercopy_fallback,
> +               .maxlen         = sizeof(int),
> +               .mode           = 0600,
> +               .proc_handler   = usercopy_fallback_handler,
> +               .extra1         = &zero,
> +               .extra2         = &one,
> +       },
>  #endif
>         { }
>  };
> diff --git a/mm/memory.c b/mm/memory.c
> index ec4e15494901..29ed21083a8d 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -104,6 +104,11 @@ EXPORT_SYMBOL(mem_map);
>  void *high_memory;
>  EXPORT_SYMBOL(high_memory);
>
> +#ifdef CONFIG_HARDENED_USERCOPY
> +/* Permit non-whitelisted usercopy targets */
> +bool usercopy_fallback = IS_ENABLED(CONFIG_HARDENED_USERCOPY_FALLBACK);
> +#endif
> +
>  /*
>   * Randomize the address space (stacks, mmaps, brk, etc.).
>   *
> @@ -4668,3 +4673,15 @@ void ptlock_free(struct page *page)
>         kmem_cache_free(page_ptl_cachep, page->ptl);
>  }
>  #endif
> +
> +#ifdef CONFIG_HARDENED_USERCOPY
> +int usercopy_fallback_handler(struct ctl_table *table, int write,
> +                           void __user *buffer, size_t *length, loff_t *ppos)
> +{
> +       /* Don't allow re-enabling fallback if it's been disabled */
> +       if (write && !usercopy_fallback)
> +               return -EINVAL;
> +
> +       return proc_dointvec_minmax(table, write, buffer, length, ppos);
> +}
> +#endif
> diff --git a/mm/slab.c b/mm/slab.c
> index 23c53dc46cd6..6afcac1d8b97 100644
> --- a/mm/slab.c
> +++ b/mm/slab.c
> @@ -4435,21 +4435,19 @@ const char *__check_heap_object(const void *ptr, unsigned long n,
>         if (offset < cachep->useroffset ||
>             offset - cachep->useroffset > cachep->usersize ||
>             n > cachep->useroffset - offset + cachep->usersize) {
> -#ifdef CONFIG_HARDENED_USERCOPY_FALLBACK
>                 /*
>                  * If no whitelist exists, and FALLBACK is set, produce
>                  * a warning instead of rejecting the copy. This is intended
>                  * to be a temporary method to find any missing usercopy
>                  * whitelists.
>                  */
> -               if (cachep->usersize == 0 &&
> +               if (usercopy_fallback && cachep->usersize == 0 &&
>                     offset <= cachep->object_size &&
>                     n <= cachep->object_size - offset) {
>                         WARN_ONCE(1, "unexpected usercopy without slab whitelist from %s offset %lu size %lu",
>                                   cachep->name, offset, n);
>                         return NULL;
>                 }
> -#endif
>                 return cachep->name;
>         }
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 82e7f15162a8..e664c6e715e6 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -3867,7 +3867,6 @@ const char *__check_heap_object(const void *ptr, unsigned long n,
>         if (offset < s->useroffset ||
>             offset - s->useroffset > s->usersize ||
>             n > s->useroffset - offset + s->usersize) {
> -#ifdef CONFIG_HARDENED_USERCOPY_FALLBACK
>                 size_t object_size;
>
>                 object_size = slab_ksize(s);
> @@ -3878,13 +3877,12 @@ const char *__check_heap_object(const void *ptr, unsigned long n,
>                  * to be a temporary method to find any missing usercopy
>                  * whitelists.
>                  */
> -               if (s->usersize == 0 &&
> +               if (usercopy_fallback && s->usersize == 0 &&
>                     (offset <= object_size && n <= object_size - offset)) {
>                         WARN_ONCE(1, "unexpected usercopy without slab whitelist from %s offset %lu size %lu",
>                                   s->name, offset, n);
>                         return NULL;
>                 }
> -#endif
>                 return s->name;
>         }
>
> --
> 2.15.0.448.gf294e3d99a-goog
>



-- 
Kees Cook
Pixel Security

Powered by blists - more mailing lists

Your e-mail address:

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.