linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Andrey Konovalov <andreyknvl@google.com>
To: Qian Cai <cai@lca.pw>
Cc: Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Alexander Potapenko <glider@google.com>,
	 Dmitry Vyukov <dvyukov@google.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	 Christoph Lameter <cl@linux.com>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	 Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	 kasan-dev <kasan-dev@googlegroups.com>,
	 Linux Memory Management List <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	 Vincenzo Frascino <vincenzo.frascino@arm.com>,
	Kostya Serebryany <kcc@google.com>,
	 Evgeniy Stepanov <eugenis@google.com>
Subject: Re: [PATCH 5/5] kasan, slub: fix conflicts with CONFIG_SLAB_FREELIST_HARDENED
Date: Tue, 12 Feb 2019 14:26:58 +0100	[thread overview]
Message-ID: <CAAeHK+zv5=oHJQg-bx7-tiD9197J7wdMeeRSgaxAfJjXEs3EyA@mail.gmail.com> (raw)
In-Reply-To: <4bc08cee-cb49-885d-ef8a-84b188d3b5b3@lca.pw>

On Tue, Feb 12, 2019 at 3:43 AM Qian Cai <cai@lca.pw> wrote:
>
>
>
> On 2/11/19 4:59 PM, Andrey Konovalov wrote:
> > CONFIG_SLAB_FREELIST_HARDENED hashes freelist pointer with the address
> > of the object where the pointer gets stored. With tag based KASAN we don't
> > account for that when building freelist, as we call set_freepointer() with
> > the first argument untagged. This patch changes the code to properly
> > propagate tags throughout the loop.
> >
> > Reported-by: Qian Cai <cai@lca.pw>
> > Signed-off-by: Andrey Konovalov <andreyknvl@google.com>
> > ---
> >  mm/slub.c | 20 +++++++-------------
> >  1 file changed, 7 insertions(+), 13 deletions(-)
> >
> > diff --git a/mm/slub.c b/mm/slub.c
> > index ce874a5c9ee7..0d32f8d30752 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -303,11 +303,6 @@ static inline void set_freepointer(struct kmem_cache *s, void *object, void *fp)
> >               __p < (__addr) + (__objects) * (__s)->size; \
> >               __p += (__s)->size)
> >
> > -#define for_each_object_idx(__p, __idx, __s, __addr, __objects) \
> > -     for (__p = fixup_red_left(__s, __addr), __idx = 1; \
> > -             __idx <= __objects; \
> > -             __p += (__s)->size, __idx++)
> > -
> >  /* Determine object index from a given position */
> >  static inline unsigned int slab_index(void *p, struct kmem_cache *s, void *addr)
> >  {
> > @@ -1655,17 +1650,16 @@ static struct page *allocate_slab(struct kmem_cache *s, gfp_t flags, int node)
> >       shuffle = shuffle_freelist(s, page);
> >
> >       if (!shuffle) {
> > -             for_each_object_idx(p, idx, s, start, page->objects) {
> > -                     if (likely(idx < page->objects)) {
> > -                             next = p + s->size;
> > -                             next = setup_object(s, page, next);
> > -                             set_freepointer(s, p, next);
> > -                     } else
> > -                             set_freepointer(s, p, NULL);
> > -             }
> >               start = fixup_red_left(s, start);
> >               start = setup_object(s, page, start);
> >               page->freelist = start;
> > +             for (idx = 0, p = start; idx < page->objects - 1; idx++) {
> > +                     next = p + s->size;
> > +                     next = setup_object(s, page, next);
> > +                     set_freepointer(s, p, next);
> > +                     p = next;
> > +             }
> > +             set_freepointer(s, p, NULL);
> >       }
> >
> >       page->inuse = page->objects;
> >
>
> Well, this one patch does not work here, as it throws endless errors below
> during boot. Still need this patch to fix it.

Hm, did you apply all 6 patches (the one that you sent and these five)?

>
> https://marc.info/?l=linux-mm&m=154955366113951&w=2
>
> [   85.744772] BUG kmemleak_object (Tainted: G    B        L   ): Freepointer
> corrupt
> [   85.744776]
> -----------------------------------------------------------------------------
> [   85.744776]
> [   85.744788] INFO: Allocated in create_object+0x88/0x9c8 age=2564 cpu=153 pid=1
> [   85.744797]  kmem_cache_alloc+0x39c/0x4ec
> [   85.744803]  create_object+0x88/0x9c8
> [   85.744811]  kmemleak_alloc+0xbc/0x180
> [   85.744818]  kmem_cache_alloc+0x3ec/0x4ec
> [   85.744825]  acpi_ut_create_generic_state+0x64/0xc4
> [   85.744832]  acpi_ut_create_pkg_state+0x24/0x1c8
> [   85.744840]  acpi_ut_walk_package_tree+0x268/0x564
> [   85.744848]  acpi_ns_init_one_package+0x80/0x114
> [   85.744856]  acpi_ns_init_one_object+0x214/0x3d8
> [   85.744862]  acpi_ns_walk_namespace+0x288/0x384
> [   85.744869]  acpi_walk_namespace+0xac/0xe8
> [   85.744877]  acpi_ns_initialize_objects+0x50/0x98
> [   85.744883]  acpi_load_tables+0xac/0x120
> [   85.744891]  acpi_init+0x128/0x850
> [   85.744898]  do_one_initcall+0x3ac/0x8c0
> [   85.744906]  kernel_init_freeable+0xcdc/0x1104
> [   85.744916] INFO: Freed in free_object_rcu+0x200/0x228 age=3 cpu=153 pid=0
> [   85.744923]  free_object_rcu+0x200/0x228
> [   85.744931]  rcu_process_callbacks+0xb00/0x12c0
> [   85.744937]  __do_softirq+0x644/0xfd0
> [   85.744944]  irq_exit+0x29c/0x370
> [   85.744952]  __handle_domain_irq+0xe0/0x1c4
> [   85.744958]  gic_handle_irq+0x1c4/0x3b0
> [   85.744964]  el1_irq+0xb0/0x140
> [   85.744971]  arch_cpu_idle+0x26c/0x594
> [   85.744978]  default_idle_call+0x44/0x5c
> [   85.744985]  do_idle+0x180/0x260
> [   85.744993]  cpu_startup_entry+0x24/0x28
> [   85.745001]  secondary_start_kernel+0x36c/0x440
> [   85.745009] INFO: Slab 0x(____ptrval____) objects=91 used=0
> fp=0x(____ptrval____) flags=0x17ffffffc000200
> [   85.745015] INFO: Object 0x(____ptrval____) @offset=35296 fp=0x(____ptrval____)
>
> kkkkk4.226750] Redzone (____ptrval____): bb bb bb bb bb bb bb bb bb bb bb bb bb
> bb bb bb  ................
> [   84.22[   84.226765] ORedzone (____ptrptrval____): 5a worker/223:0 Tainted: G
>    B        L    5.0.0-rc6+ #36
> [   84.226790] Hardware name: HPE Apollo 70             /C01_APACHE_MB         ,
> BIOS L50_5.13_1.0.6 07/10/2018
> [   84.226798] Workqueue: events free_obj_work
> [   84.226802] Call trace:
> [   84.226809]  dump_backtrace+0x0/0x450
> [   84.226815]  show_stack+0x20/0x2c
> [   84.226822]  __dump_stack+0x20/0x28
> [   84.226828]  dump_stack+0xa0/0xfc
> [   84.226835]  print_trailer+0x1a8/0x1bc
> [   84.226842]  object_err+0x40/0x50
> [   84.226848]  check_object+0x214/0x2b8
> [   84.226854]  __free_slab+0x9c/0x31c
> [   84.226860]  discard_slab+0x78/0xa8
> [   84.226866]  kmem_cache_free+0x99c/0x9f0
> [   84.226873]  free_obj_work+0x92c/0xa44
> [   84.226879]  process_one_work+0x894/0x1280
> [   84.226885]  worker_thread+0x684/0xa1c
> [   84.226892]  kthread+0x2cc/0x2e8
> [   84.226898]  ret_from_fork+0x10/0x18
> [   84.229197]


  reply	other threads:[~2019-02-12 13:27 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-02-11 21:59 [PATCH 0/5] kasan: more tag based mode fixes Andrey Konovalov
2019-02-11 21:59 ` [PATCH 1/5] kasan: fix assigning tags twice Andrey Konovalov
2019-02-11 21:59 ` [PATCH 2/5] kasan, kmemleak: pass tagged pointers to kmemleak Andrey Konovalov
2019-02-12 15:56   ` Vincenzo Frascino
2019-02-13 13:07     ` Andrey Konovalov
2019-02-11 21:59 ` [PATCH 3/5] kmemleak: account for tagged pointers when calculating pointer range Andrey Konovalov
2019-02-15 14:05   ` Catalin Marinas
2019-02-11 21:59 ` [PATCH 4/5] kasan, slub: move kasan_poison_slab hook before page_address Andrey Konovalov
2019-02-12 21:12   ` Andrew Morton
2019-02-13 13:25     ` Andrey Konovalov
2019-02-11 21:59 ` [PATCH 5/5] kasan, slub: fix conflicts with CONFIG_SLAB_FREELIST_HARDENED Andrey Konovalov
2019-02-12  2:43   ` Qian Cai
2019-02-12 13:26     ` Andrey Konovalov [this message]
2019-02-12 13:43       ` Qian Cai
2019-02-12 14:42         ` Andrey Konovalov
2019-02-12 16:07           ` Qian Cai
2019-02-13  2:15             ` Qian Cai

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CAAeHK+zv5=oHJQg-bx7-tiD9197J7wdMeeRSgaxAfJjXEs3EyA@mail.gmail.com' \
    --to=andreyknvl@google.com \
    --cc=akpm@linux-foundation.org \
    --cc=aryabinin@virtuozzo.com \
    --cc=cai@lca.pw \
    --cc=catalin.marinas@arm.com \
    --cc=cl@linux.com \
    --cc=dvyukov@google.com \
    --cc=eugenis@google.com \
    --cc=glider@google.com \
    --cc=iamjoonsoo.kim@lge.com \
    --cc=kasan-dev@googlegroups.com \
    --cc=kcc@google.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=penberg@kernel.org \
    --cc=rientjes@google.com \
    --cc=vincenzo.frascino@arm.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox