linux-kernel - Re: [PATCH v3 4/5] mm/slab: Refactor common ksize KASAN logic into slab

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Wed, 26 Jun 2019 15:56:50 -0700
From:   Andrew Morton <akpm@...ux-foundation.org>
To:     Marco Elver <elver@...gle.com>
Cc:     linux-kernel@...r.kernel.org,
        Andrey Ryabinin <aryabinin@...tuozzo.com>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Alexander Potapenko <glider@...gle.com>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        Christoph Lameter <cl@...ux.com>,
        Pekka Enberg <penberg@...nel.org>,
        David Rientjes <rientjes@...gle.com>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Mark Rutland <mark.rutland@....com>,
        kasan-dev@...glegroups.com, linux-mm@...ck.org,
        Kees Cook <keescook@...omium.org>
Subject: Re: [PATCH v3 4/5] mm/slab: Refactor common ksize KASAN logic into
 slab_common.c

On Wed, 26 Jun 2019 16:20:13 +0200 Marco Elver <elver@...gle.com> wrote:

> This refactors common code of ksize() between the various allocators
> into slab_common.c: __ksize() is the allocator-specific implementation
> without instrumentation, whereas ksize() includes the required KASAN
> logic.
> 
> ...
>
>  /**
> - * ksize - get the actual amount of memory allocated for a given object
> - * @objp: Pointer to the object
> + * __ksize -- Uninstrumented ksize.
>   *
> - * kmalloc may internally round up allocations and return more memory
> - * than requested. ksize() can be used to determine the actual amount of
> - * memory allocated. The caller may use this additional memory, even though
> - * a smaller amount of memory was initially specified with the kmalloc call.
> - * The caller must guarantee that objp points to a valid object previously
> - * allocated with either kmalloc() or kmem_cache_alloc(). The object
> - * must not be freed during the duration of the call.
> - *
> - * Return: size of the actual memory used by @objp in bytes
> + * Unlike ksize(), __ksize() is uninstrumented, and does not provide the same
> + * safety checks as ksize() with KASAN instrumentation enabled.
>   */
> -size_t ksize(const void *objp)
> +size_t __ksize(const void *objp)
>  {
> -	size_t size;
> -
>  	BUG_ON(!objp);
>  	if (unlikely(objp == ZERO_SIZE_PTR))
>  		return 0;
>  
> -	size = virt_to_cache(objp)->object_size;
> -	/* We assume that ksize callers could use the whole allocated area,
> -	 * so we need to unpoison this area.
> -	 */
> -	kasan_unpoison_shadow(objp, size);
> -
> -	return size;
> +	return virt_to_cache(objp)->object_size;
>  }

This conflicts with Kees's "mm/slab: sanity-check page type when
looking up cache". 
https://ozlabs.org/~akpm/mmots/broken-out/mm-slab-sanity-check-page-type-when-looking-up-cache.patch

Here's what I ended up with:

/**
 * __ksize -- Uninstrumented ksize.
 *
 * Unlike ksize(), __ksize() is uninstrumented, and does not provide the same
 * safety checks as ksize() with KASAN instrumentation enabled.
 */
size_t __ksize(const void *objp)
{
	size_t size;
	struct kmem_cache *c;

	BUG_ON(!objp);
	if (unlikely(objp == ZERO_SIZE_PTR))
		return 0;

	c = virt_to_cache(objp);
	size = c ? c->object_size : 0;

	return size;
}
EXPORT_SYMBOL(__ksize);

> --- a/mm/slab_common.c
> +++ b/mm/slab_common.c
> @@ -1597,6 +1597,32 @@ void kzfree(const void *p)
>  }
>  EXPORT_SYMBOL(kzfree);
>  
> +/**
> + * ksize - get the actual amount of memory allocated for a given object
> + * @objp: Pointer to the object
> + *
> + * kmalloc may internally round up allocations and return more memory
> + * than requested. ksize() can be used to determine the actual amount of
> + * memory allocated. The caller may use this additional memory, even though
> + * a smaller amount of memory was initially specified with the kmalloc call.
> + * The caller must guarantee that objp points to a valid object previously
> + * allocated with either kmalloc() or kmem_cache_alloc(). The object
> + * must not be freed during the duration of the call.
> + *
> + * Return: size of the actual memory used by @objp in bytes
> + */
> +size_t ksize(const void *objp)
> +{
> +	size_t size = __ksize(objp);
> +	/*
> +	 * We assume that ksize callers could use whole allocated area,
> +	 * so we need to unpoison this area.
> +	 */
> +	kasan_unpoison_shadow(objp, size);
> +	return size;
> +}
> +EXPORT_SYMBOL(ksize);

That looks OK still.