Re: [PATCH v3 01/31] usercopy: Prepare for usercopy whitelisting

From: Christopher Lameter
Date: Thu Sep 21 2017 - 11:21:25 EST


On Wed, 20 Sep 2017, Kees Cook wrote:

> diff --git a/include/linux/stddef.h b/include/linux/stddef.h
> index 9c61c7cda936..f00355086fb2 100644
> --- a/include/linux/stddef.h
> +++ b/include/linux/stddef.h
> @@ -18,6 +18,8 @@ enum {
> #define offsetof(TYPE, MEMBER) ((size_t)&((TYPE *)0)->MEMBER)
> #endif
>
> +#define sizeof_field(structure, field) sizeof((((structure *)0)->field))
> +
> /**
> * offsetofend(TYPE, MEMBER)
> *

Hmmm.. Is that really necessary? Code knows the type of field and can
use sizeof type.

Also this is a non slab change hidden in the patchset.

> diff --git a/mm/slab_common.c b/mm/slab_common.c
> index 904a83be82de..36408f5f2a34 100644
> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -272,6 +272,9 @@ int slab_unmergeable(struct kmem_cache *s)
> if (s->ctor)
> return 1;
>
> + if (s->usersize)
> + return 1;
> +
> /*
> * We may have set a slab to be unmergeable during bootstrap.
> */

This will ultimately make all slabs unmergeable at the end of your
patchset? Lots of space will be wasted. Is there any way to make this
feature optional?

#ifdef CONFIG_HARDENED around this?


> @@ -491,6 +509,15 @@ kmem_cache_create(const char *name, size_t size, size_t align,
> }
> return s;
> }
> +EXPORT_SYMBOL(kmem_cache_create_usercopy);
> +
> +struct kmem_cache *
> +kmem_cache_create(const char *name, size_t size, size_t align,
> + unsigned long flags, void (*ctor)(void *))
> +{
> + return kmem_cache_create_usercopy(name, size, align, flags, 0, size,
> + ctor);
> +}
> EXPORT_SYMBOL(kmem_cache_create);

Well this makes the slab created unmergeable.

> @@ -897,7 +927,7 @@ struct kmem_cache *__init create_kmalloc_cache(const char *name, size_t size,
> if (!s)
> panic("Out of memory when creating slab %s\n", name);
>
> - create_boot_cache(s, name, size, flags);
> + create_boot_cache(s, name, size, flags, 0, size);

Ok this makes the kmalloc array unmergeable.

> @@ -5081,6 +5081,12 @@ static ssize_t cache_dma_show(struct kmem_cache *s, char *buf)
> SLAB_ATTR_RO(cache_dma);
> #endif
>
> +static ssize_t usersize_show(struct kmem_cache *s, char *buf)
> +{
> + return sprintf(buf, "%zu\n", s->usersize);
> +}
> +SLAB_ATTR_RO(usersize);
> +
> static ssize_t destroy_by_rcu_show(struct kmem_cache *s, char *buf)
> {
> return sprintf(buf, "%d\n", !!(s->flags & SLAB_TYPESAFE_BY_RCU));
> @@ -5455,6 +5461,7 @@ static struct attribute *slab_attrs[] = {
> #ifdef CONFIG_FAILSLAB
> &failslab_attr.attr,
> #endif
> + &usersize_attr.attr,

So useroffset is not exposed?