linux-kernel - Re: [PATCH] slub: Avoid redzone when choosing freepointer location

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CANpmjNPORrwPkqByhtKgVoh5=KHkpYHM0zRv6O1RgONmx3ZqOg@mail.gmail.com>
Date:   Wed, 15 Apr 2020 20:07:57 +0200
From:   Marco Elver <elver@...gle.com>
To:     Kees Cook <keescook@...omium.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Christoph Lameter <cl@...ux.com>,
        Pekka Enberg <penberg@...nel.org>,
        David Rientjes <rientjes@...gle.com>,
        Joonsoo Kim <iamjoonsoo.kim@....com>,
        Linux Memory Management List <linux-mm@...ck.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH] slub: Avoid redzone when choosing freepointer location

On Wed, 15 Apr 2020 at 19:55, Kees Cook <keescook@...omium.org> wrote:
>
> Marco Elver reported system crashes when booting with "slub_debug=Z".
> The freepointer location (s->offset) was not taking into account that
> the "inuse" size that includes the redzone area should not be used by
> the freelist pointer. Change the calculation to save the area of the
> object that an inline freepointer may be written into.
>
> Reported-by: Marco Elver <elver@...gle.com>
> Link: https://lore.kernel.org/linux-mm/20200415164726.GA234932@google.com
> Fixes: 3202fa62fb43 ("slub: relocate freelist pointer to middle of object")
> Signed-off-by: Kees Cook <keescook@...omium.org>

Works for me, thank you!

Tested-by: Marco Elver <elver@...gle.com>

> ---
>  mm/slub.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
>
> diff --git a/mm/slub.c b/mm/slub.c
> index 332d4b459a90..9bf44955c4f1 100644
> --- a/mm/slub.c
> +++ b/mm/slub.c
> @@ -3533,6 +3533,7 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order)
>  {
>         slab_flags_t flags = s->flags;
>         unsigned int size = s->object_size;
> +       unsigned int freepointer_area;
>         unsigned int order;
>
>         /*
> @@ -3541,6 +3542,13 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order)
>          * the possible location of the free pointer.
>          */
>         size = ALIGN(size, sizeof(void *));
> +       /*
> +        * This is the area of the object where a freepointer can be
> +        * safely written. If redzoning adds more to the inuse size, we
> +        * can't use that portion for writing the freepointer, so
> +        * s->offset must be limited within this for the general case.
> +        */
> +       freepointer_area = size;
>
>  #ifdef CONFIG_SLUB_DEBUG
>         /*
> @@ -3582,13 +3590,13 @@ static int calculate_sizes(struct kmem_cache *s, int forced_order)
>                  */
>                 s->offset = size;
>                 size += sizeof(void *);
> -       } else if (size > sizeof(void *)) {
> +       } else if (freepointer_area > sizeof(void *)) {
>                 /*
>                  * Store freelist pointer near middle of object to keep
>                  * it away from the edges of the object to avoid small
>                  * sized over/underflows from neighboring allocations.
>                  */
> -               s->offset = ALIGN(size / 2, sizeof(void *));
> +               s->offset = ALIGN(freepointer_area / 2, sizeof(void *));
>         }
>
>  #ifdef CONFIG_SLUB_DEBUG
> --
> 2.20.1
>
>
> --
> Kees Cook