Skip to content

Commit

Permalink
kasan: fix object remaining in offline per-cpu quarantine
Browse files Browse the repository at this point in the history
We hit this issue in our internal test.  When enabling generic kasan, a
kfree()'d object is put into per-cpu quarantine first.  If the cpu goes
offline, object still remains in the per-cpu quarantine.  If we call
kmem_cache_destroy() now, slub will report "Objects remaining" error.

  =============================================================================
  BUG test_module_slab (Not tainted): Objects remaining in test_module_slab on __kmem_cache_shutdown()
  -----------------------------------------------------------------------------

  Disabling lock debugging due to kernel taint
  INFO: Slab 0x(____ptrval____) objects=34 used=1 fp=0x(____ptrval____) flags=0x2ffff00000010200
  CPU: 3 PID: 176 Comm: cat Tainted: G    B             5.10.0-rc1-00007-g4525c8781ec0-dirty #10
  Hardware name: linux,dummy-virt (DT)
  Call trace:
     dump_backtrace+0x0/0x2b0
     show_stack+0x18/0x68
     dump_stack+0xfc/0x168
     slab_err+0xac/0xd4
     __kmem_cache_shutdown+0x1e4/0x3c8
     kmem_cache_destroy+0x68/0x130
     test_version_show+0x84/0xf0
     module_attr_show+0x40/0x60
     sysfs_kf_seq_show+0x128/0x1c0
     kernfs_seq_show+0xa0/0xb8
     seq_read+0x1f0/0x7e8
     kernfs_fop_read+0x70/0x338
     vfs_read+0xe4/0x250
     ksys_read+0xc8/0x180
     __arm64_sys_read+0x44/0x58
     el0_svc_common.constprop.0+0xac/0x228
     do_el0_svc+0x38/0xa0
     el0_sync_handler+0x170/0x178
     el0_sync+0x174/0x180
  INFO: Object 0x(____ptrval____) @offset=15848
  INFO: Allocated in test_version_show+0x98/0xf0 age=8188 cpu=6 pid=172
     stack_trace_save+0x9c/0xd0
     set_track+0x64/0xf0
     alloc_debug_processing+0x104/0x1a0
     ___slab_alloc+0x628/0x648
     __slab_alloc.isra.0+0x2c/0x58
     kmem_cache_alloc+0x560/0x588
     test_version_show+0x98/0xf0
     module_attr_show+0x40/0x60
     sysfs_kf_seq_show+0x128/0x1c0
     kernfs_seq_show+0xa0/0xb8
     seq_read+0x1f0/0x7e8
     kernfs_fop_read+0x70/0x338
     vfs_read+0xe4/0x250
     ksys_read+0xc8/0x180
     __arm64_sys_read+0x44/0x58
     el0_svc_common.constprop.0+0xac/0x228
  kmem_cache_destroy test_module_slab: Slab cache still has objects

Register a cpu hotplug function to remove all objects in the offline
per-cpu quarantine when cpu is going offline.  Set a per-cpu variable to
indicate this cpu is offline.

[[email protected]: fix slab double free when cpu-hotplug]
  Link: https://lkml.kernel.org/r/[email protected]

Link: https://lkml.kernel.org/r/[email protected]
Signed-off-by: Kuan-Ying Lee <[email protected]>
Signed-off-by: Zqiang <[email protected]>
Suggested-by: Dmitry Vyukov <[email protected]>
Reported-by: Guangye Yang <[email protected]>
Reviewed-by: Dmitry Vyukov <[email protected]>
Cc: Andrey Ryabinin <[email protected]>
Cc: Alexander Potapenko <[email protected]>
Cc: Matthias Brugger <[email protected]>
Cc: Nicholas Tang <[email protected]>
Cc: Miles Chen <[email protected]>
Cc: Qian Cai <[email protected]>
Cc: Stephen Rothwell <[email protected]>
Signed-off-by: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
Kuan-Ying Lee authored and torvalds committed Dec 11, 2020
1 parent 6e7b64b commit 6c82d45
Showing 1 changed file with 39 additions and 0 deletions.
39 changes: 39 additions & 0 deletions mm/kasan/quarantine.c
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
#include <linux/srcu.h>
#include <linux/string.h>
#include <linux/types.h>
#include <linux/cpuhotplug.h>

#include "../slab.h"
#include "kasan.h"
Expand All @@ -43,6 +44,7 @@ struct qlist_head {
struct qlist_node *head;
struct qlist_node *tail;
size_t bytes;
bool offline;
};

#define QLIST_INIT { NULL, NULL, 0 }
Expand Down Expand Up @@ -188,6 +190,10 @@ void quarantine_put(struct kasan_free_meta *info, struct kmem_cache *cache)
local_irq_save(flags);

q = this_cpu_ptr(&cpu_quarantine);
if (q->offline) {
local_irq_restore(flags);
return;
}
qlist_put(q, &info->quarantine_link, cache->size);
if (unlikely(q->bytes > QUARANTINE_PERCPU_SIZE)) {
qlist_move_all(q, &temp);
Expand Down Expand Up @@ -328,3 +334,36 @@ void quarantine_remove_cache(struct kmem_cache *cache)

synchronize_srcu(&remove_cache_srcu);
}

static int kasan_cpu_online(unsigned int cpu)
{
this_cpu_ptr(&cpu_quarantine)->offline = false;
return 0;
}

static int kasan_cpu_offline(unsigned int cpu)
{
struct qlist_head *q;

q = this_cpu_ptr(&cpu_quarantine);
/* Ensure the ordering between the writing to q->offline and
* qlist_free_all. Otherwise, cpu_quarantine may be corrupted
* by interrupt.
*/
WRITE_ONCE(q->offline, true);
barrier();
qlist_free_all(q, NULL);
return 0;
}

static int __init kasan_cpu_quarantine_init(void)
{
int ret = 0;

ret = cpuhp_setup_state(CPUHP_AP_ONLINE_DYN, "mm/kasan:online",
kasan_cpu_online, kasan_cpu_offline);
if (ret < 0)
pr_err("kasan cpu quarantine register failed [%d]\n", ret);
return ret;
}
late_initcall(kasan_cpu_quarantine_init);

0 comments on commit 6c82d45

Please sign in to comment.