From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-14.4 required=3.0 tests=DKIMWL_WL_MED,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED, USER_IN_DEF_DKIM_WL autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 958D7CA9EA1 for ; Fri, 18 Oct 2019 13:55:22 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 1659D222BD for ; Fri, 18 Oct 2019 13:55:21 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="tG6zWA4N" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 1659D222BD Authentication-Results: mail.kernel.org; dmarc=fail (p=reject dis=none) header.from=google.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 7493A8E0005; Fri, 18 Oct 2019 09:55:21 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 6F9028E0003; Fri, 18 Oct 2019 09:55:21 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 610018E0005; Fri, 18 Oct 2019 09:55:21 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0061.hostedemail.com [216.40.44.61]) by kanga.kvack.org (Postfix) with ESMTP id 3ECEC8E0003 for ; Fri, 18 Oct 2019 09:55:21 -0400 (EDT) Received: from smtpin26.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay04.hostedemail.com (Postfix) with SMTP id BC21C8140 for ; Fri, 18 Oct 2019 13:55:20 +0000 (UTC) X-FDA: 76057052400.26.hall67_85e35094f7c45 X-HE-Tag: hall67_85e35094f7c45 X-Filterd-Recvd-Size: 12399 Received: from mail-wr1-f67.google.com (mail-wr1-f67.google.com [209.85.221.67]) by imf45.hostedemail.com (Postfix) with ESMTP for ; Fri, 18 Oct 2019 13:55:19 +0000 (UTC) Received: by mail-wr1-f67.google.com with SMTP id q13so1400477wrs.12 for ; Fri, 18 Oct 2019 06:55:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=IMqHL4torXpnlrWLkhuB+kDUgcIkr9TH9xwyEc8Z2oM=; b=tG6zWA4N5IInwtuoImdTEmgHBH8mmnf2ZXZ3Gns6ws2Rz8oJl2tMdkNq9pbzJAD6Tv H8jR0mu2swuXoy3zJOXHJSVYKjLvAu+3P/tJwKhvT9GYg0gMoFcGfRcwJF/xichtPF9m S8IxA56vDE1xO6JurusI84HP/nsmHqFhgO4/GBKsLuuetvlmpbT3iwc5Y5krfQueMHOH pqKudrX8YMfKGs61bVM2tx4TX2zdh34iB8kA9If09VaiGlpioBlVbkHjnGgKjFk3BZn6 MxI7SKHwV9Mn55AtzIZ5Ev/3TXqZq9/MO8rJudf51cwNxXuxHLpAbO7XKNt/SsevXmHx z4+w== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=IMqHL4torXpnlrWLkhuB+kDUgcIkr9TH9xwyEc8Z2oM=; b=T9eaIjqrwsGOgZA7lCdC2fjlbQQgO5XVU0MNA+gHFGK+LmucaxB6JnD7yPIy+ab6NX Fe+FOzP9IsNA8DOAiycTlfFThMOhcfeYUyVg6LLPQ3JMK4WqzLBngs0gcegfmKnsEprN 71TR8TZ8y6jMGhI8FmPryMdfot8hkODk0bcJ/ZJgzw55bYKHCbo+5cRGOYanfiumtR8k VjNRvcesT5BX5QsEGKtxuOrvR4OTkdIIGNuJsuH5WL+WNP0PFizCo15PZfnyDqvSD2Qw 41rZBuaNF0Q7232qxMaQRFTHhtvAm211U8Ds/PSGDUtf3JLOVUv6EYK1Vi2+SBCPxkIa 5hMQ== X-Gm-Message-State: APjAAAWzrgtAfCMLD9n6lGJaRatiMyaz40qBvvhn2t+ieXfx3+3anVAr Wq+A5p21kYcBsqQikBBaV9mkXBNI5OELr46wBTI7/w== X-Google-Smtp-Source: APXvYqyPEFrJyiaeh4C9azjc2qHCypjNgW8aj5GAyGlqciGReWFuIN4NlWlN2qjgfJnnrPnj4+VhoTiNcjcYOppd+cQ= X-Received: by 2002:a5d:4b42:: with SMTP id w2mr7833974wrs.360.1571406918273; Fri, 18 Oct 2019 06:55:18 -0700 (PDT) MIME-Version: 1.0 References: <20191018094304.37056-1-glider@google.com> <20191018094304.37056-19-glider@google.com> <1571404943.5937.71.camel@lca.pw> <1571406117.5937.73.camel@lca.pw> In-Reply-To: <1571406117.5937.73.camel@lca.pw> From: Alexander Potapenko Date: Fri, 18 Oct 2019 15:55:06 +0200 Message-ID: Subject: Re: [PATCH RFC v1 18/26] kmsan: mm: call KMSAN hooks from SLUB code To: Qian Cai Cc: Andrew Morton , Vegard Nossum , Dmitry Vyukov , Linux Memory Management List Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Fri, Oct 18, 2019 at 3:42 PM Qian Cai wrote: > > On Fri, 2019-10-18 at 15:33 +0200, Alexander Potapenko wrote: > > On Fri, Oct 18, 2019 at 3:22 PM Qian Cai wrote: > > > > > > On Fri, 2019-10-18 at 11:42 +0200, glider@google.com wrote: > > > > In order to report uninitialized memory coming from heap allocation= s > > > > KMSAN has to poison them unless they're created with __GFP_ZERO. > > > > > > > > It's handy that we need KMSAN hooks in the places where > > > > init_on_alloc/init_on_free initialization is performed. > > > > > > Well, there is SLUB debug which has red zoning and poisoning checks. = What's > > > value of this patch? > > > > Sorry, are you talking about the whole patchset or just this patch? > > Just this patch. > > > Note that SLUB debug is unable to detect uninitialized values with > > bit-to-bit precision, neither have I heard of anyone using it for > > detecting uses of uninitialized memory in the kernel at all. > > The purpose of SLUB debug is to detect corruptions of freed memory. > > The point is if developers have SLUB debug enabled, all the free objects = will be > poisoned, so what's the point of checking uninitialized memory there? You are right, SLUB debug has to be handled separately. If I'm understanding correctly, right now KMSAN poisons freed memory before SLUB debug wipes it, therefore the memory will count as initialized. On the other hand, newly allocated memory is still marked as uninitialized, so a lot of bugs still remain detectable. TBH, I haven't tested KMSAN with SLUB debug good enough. Note that it's anyway a separate build that requires Clang, so right now it doesn't make much sense to combine KMSAN and SLUB debug together. > > > > > > > > Signed-off-by: Alexander Potapenko > > > > To: Alexander Potapenko > > > > Cc: Andrew Morton > > > > Cc: Vegard Nossum > > > > Cc: Dmitry Vyukov > > > > Cc: linux-mm@kvack.org > > > > --- > > > > > > > > Change-Id: I51103b7981d3aabed747d0c85cbdc85568665871 > > > > --- > > > > mm/slub.c | 37 +++++++++++++++++++++++++++++++------ > > > > 1 file changed, 31 insertions(+), 6 deletions(-) > > > > > > > > diff --git a/mm/slub.c b/mm/slub.c > > > > index 3d63ae320d31..3d6d4c63446e 100644 > > > > --- a/mm/slub.c > > > > +++ b/mm/slub.c > > > > @@ -21,6 +21,8 @@ > > > > #include > > > > #include > > > > #include > > > > +#include > > > > +#include /* KMSAN_INIT_VALUE */ > > > > #include > > > > #include > > > > #include > > > > @@ -285,17 +287,27 @@ static void prefetch_freepointer(const struct= kmem_cache *s, void *object) > > > > prefetch(object + s->offset); > > > > } > > > > > > > > +/* > > > > + * When running under KMSAN, get_freepointer_safe() may return an = uninitialized > > > > + * pointer value in the case the current thread loses the race for= the next > > > > + * memory chunk in the freelist. In that case this_cpu_cmpxchg_dou= ble() in > > > > + * slab_alloc_node() will fail, so the uninitialized value won't b= e used, but > > > > + * KMSAN will still check all arguments of cmpxchg because of impe= rfect > > > > + * handling of inline assembly. > > > > + * To work around this problem, use KMSAN_INIT_VALUE() to force in= itialize the > > > > + * return value of get_freepointer_safe(). > > > > + */ > > > > static inline void *get_freepointer_safe(struct kmem_cache *s, voi= d *object) > > > > { > > > > unsigned long freepointer_addr; > > > > void *p; > > > > > > > > if (!debug_pagealloc_enabled()) > > > > - return get_freepointer(s, object); > > > > + return KMSAN_INIT_VALUE(get_freepointer(s, object)); > > > > > > > > freepointer_addr =3D (unsigned long)object + s->offset; > > > > probe_kernel_read(&p, (void **)freepointer_addr, sizeof(p)); > > > > - return freelist_ptr(s, p, freepointer_addr); > > > > + return KMSAN_INIT_VALUE(freelist_ptr(s, p, freepointer_addr))= ; > > > > } > > > > > > > > static inline void set_freepointer(struct kmem_cache *s, void *obj= ect, void *fp) > > > > @@ -1390,6 +1402,7 @@ static inline void *kmalloc_large_node_hook(v= oid *ptr, size_t size, gfp_t flags) > > > > ptr =3D kasan_kmalloc_large(ptr, size, flags); > > > > /* As ptr might get tagged, call kmemleak hook after KASAN. *= / > > > > kmemleak_alloc(ptr, size, 1, flags); > > > > + kmsan_kmalloc_large(ptr, size, flags); > > > > return ptr; > > > > } > > > > > > > > @@ -1397,6 +1410,7 @@ static __always_inline void kfree_hook(void *= x) > > > > { > > > > kmemleak_free(x); > > > > kasan_kfree_large(x, _RET_IP_); > > > > + kmsan_kfree_large(x); > > > > } > > > > > > > > static __always_inline bool slab_free_hook(struct kmem_cache *s, v= oid *x) > > > > @@ -1453,6 +1467,12 @@ static inline bool slab_free_freelist_hook(s= truct kmem_cache *s, > > > > } while (object !=3D old_tail); > > > > } > > > > > > > > + do { > > > > + object =3D next; > > > > + next =3D get_freepointer(s, object); > > > > + kmsan_slab_free(s, object); > > > > + } while (object !=3D old_tail); > > > > + > > > > /* > > > > * Compiler cannot detect this function can be removed if slab_fre= e_hook() > > > > * evaluates to nothing. Thus, catch all relevant config debug op= tions here. > > > > @@ -2769,6 +2789,7 @@ static __always_inline void *slab_alloc_node(= struct kmem_cache *s, > > > > if (unlikely(slab_want_init_on_alloc(gfpflags, s)) && object) > > > > memset(object, 0, s->object_size); > > > > > > > > + kmsan_slab_alloc(s, object, gfpflags); > > > > slab_post_alloc_hook(s, gfpflags, 1, &object); > > > > > > > > return object; > > > > @@ -2797,6 +2818,7 @@ void *kmem_cache_alloc_trace(struct kmem_cach= e *s, gfp_t gfpflags, size_t size) > > > > void *ret =3D slab_alloc(s, gfpflags, _RET_IP_); > > > > trace_kmalloc(_RET_IP_, ret, size, s->size, gfpflags); > > > > ret =3D kasan_kmalloc(s, ret, size, gfpflags); > > > > + > > > > return ret; > > > > } > > > > EXPORT_SYMBOL(kmem_cache_alloc_trace); > > > > @@ -2809,7 +2831,6 @@ void *kmem_cache_alloc_node(struct kmem_cache= *s, gfp_t gfpflags, int node) > > > > > > > > trace_kmem_cache_alloc_node(_RET_IP_, ret, > > > > s->object_size, s->size, gfpflags= , node); > > > > - > > > > return ret; > > > > } > > > > EXPORT_SYMBOL(kmem_cache_alloc_node); > > > > @@ -2825,6 +2846,7 @@ void *kmem_cache_alloc_node_trace(struct kmem= _cache *s, > > > > size, s->size, gfpflags, node); > > > > > > > > ret =3D kasan_kmalloc(s, ret, size, gfpflags); > > > > + > > > > return ret; > > > > } > > > > EXPORT_SYMBOL(kmem_cache_alloc_node_trace); > > > > @@ -3150,7 +3172,7 @@ int kmem_cache_alloc_bulk(struct kmem_cache *= s, gfp_t flags, size_t size, > > > > void **p) > > > > { > > > > struct kmem_cache_cpu *c; > > > > - int i; > > > > + int i, j; > > > > > > > > /* memcg and kmem_cache debug support */ > > > > s =3D slab_pre_alloc_hook(s, flags); > > > > @@ -3188,11 +3210,11 @@ int kmem_cache_alloc_bulk(struct kmem_cache= *s, gfp_t flags, size_t size, > > > > > > > > /* Clear memory outside IRQ disabled fastpath loop */ > > > > if (unlikely(slab_want_init_on_alloc(flags, s))) { > > > > - int j; > > > > - > > > > for (j =3D 0; j < i; j++) > > > > memset(p[j], 0, s->object_size); > > > > } > > > > + for (j =3D 0; j < i; j++) > > > > + kmsan_slab_alloc(s, p[j], flags); > > > > > > > > /* memcg and kmem_cache debug support */ > > > > slab_post_alloc_hook(s, flags, size, p); > > > > @@ -3793,6 +3815,7 @@ static int __init setup_slub_min_objects(char= *str) > > > > > > > > __setup("slub_min_objects=3D", setup_slub_min_objects); > > > > > > > > +__no_sanitize_memory > > > > void *__kmalloc(size_t size, gfp_t flags) > > > > { > > > > struct kmem_cache *s; > > > > @@ -5698,6 +5721,7 @@ static char *create_unique_id(struct kmem_cac= he *s) > > > > p +=3D sprintf(p, "%07u", s->size); > > > > > > > > BUG_ON(p > name + ID_STR_LENGTH - 1); > > > > + kmsan_unpoison_shadow(name, p - name); > > > > return name; > > > > } > > > > > > > > @@ -5847,6 +5871,7 @@ static int sysfs_slab_alias(struct kmem_cache= *s, const char *name) > > > > al->name =3D name; > > > > al->next =3D alias_list; > > > > alias_list =3D al; > > > > + kmsan_unpoison_shadow(al, sizeof(struct saved_alias)); > > > > return 0; > > > > } > > > > > > > > > > --=20 Alexander Potapenko Software Engineer Google Germany GmbH Erika-Mann-Stra=C3=9Fe, 33 80636 M=C3=BCnchen Gesch=C3=A4ftsf=C3=BChrer: Paul Manicle, Halimah DeLaine Prado Registergericht und -nummer: Hamburg, HRB 86891 Sitz der Gesellschaft: Hamburg