Skip to content

Commit

Permalink
kmsan: don't use |pc| passed to __msan_poison_alloca()
Browse files Browse the repository at this point in the history
It should be simpler to just get the necessary number of alloca stack
frames in __msan_poison_alloca().
Right now we store only the return address of __msan_poison_alloca() and
the caller, for which we need kmsan_internal_return_address().
  • Loading branch information
ramosian-glider committed Sep 19, 2018
1 parent 7037292 commit 27aa601
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 34 deletions.
34 changes: 4 additions & 30 deletions mm/kmsan/kmsan.c
Expand Up @@ -572,7 +572,7 @@ depot_stack_handle_t inline kmsan_internal_chain_origin(depot_stack_handle_t id,
if (full) {
entries[1] = kmsan_save_stack();
} else {
entries[1] = (unsigned long)__builtin_return_address(1);
entries[1] = (unsigned long)kmsan_internal_return_address(1);
}
entries[2] = id;
handle = depot_save_stack(&trace, GFP_ATOMIC);
Expand Down Expand Up @@ -905,32 +905,6 @@ void save_reporter(void *caller, void **table, int *index)
table[(*index)++] = caller;
}

// Dummy replacement for __builtin_return_address() which may crash without
// frame pointers.
inline void *return_address(int arg)
{
#ifdef CONFIG_UNWINDER_FRAME_POINTER
switch (arg) {
case 1:
return __builtin_return_address(1);
case 2:
return __builtin_return_address(2);
default:
BUG();
}
#else
unsigned long entries[1];
struct stack_trace trace = {
.nr_entries = 0,
.entries = entries,
.max_entries = 1,
.skip = arg
};
save_stack_trace(&trace);
return entries[0];
#endif
}

// |deep| is a dirty hack to skip an additional frame when calling
// kmsan_report() from kmsan_copy_to_user().
inline void kmsan_report(void *caller, depot_stack_handle_t origin,
Expand Down Expand Up @@ -970,13 +944,13 @@ inline void kmsan_report(void *caller, depot_stack_handle_t origin,
spin_lock_irqsave(&report_lock, flags);
save_reporter(caller, reporters_tbl, &reporters_index);
kmsan_pr_err("==================================================================\n");
// TODO(glider): inline this properly, avoid __builtin_return_address(1).
// TODO(glider): inline this properly
switch (reason) {
case REASON_ANY:
kmsan_pr_err("BUG: KMSAN: uninit-value in %pS\n", deep ? return_address(2) : return_address(1));
kmsan_pr_err("BUG: KMSAN: uninit-value in %pS\n", deep ? kmsan_internal_return_address(2) : kmsan_internal_return_address(1));
break;
case REASON_COPY_TO_USER:
kmsan_pr_err("BUG: KMSAN: kernel-infoleak in %pS\n", deep ? return_address(2) : return_address(1));
kmsan_pr_err("BUG: KMSAN: kernel-infoleak in %pS\n", deep ? kmsan_internal_return_address(2) : kmsan_internal_return_address(1));
break;
}
dump_stack();
Expand Down
27 changes: 27 additions & 0 deletions mm/kmsan/kmsan.h
Expand Up @@ -113,4 +113,31 @@ struct page *virt_to_page_or_null(const void *vaddr);
void *get_cea_shadow_or_null(const void *addr);
void *get_cea_origin_or_null(const void *addr);

// Dummy replacement for __builtin_return_address() which may crash without
// frame pointers.
static inline void *kmsan_internal_return_address(int arg)
{
#ifdef CONFIG_UNWINDER_FRAME_POINTER
switch (arg) {
case 1:
return __builtin_return_address(1);
case 2:
return __builtin_return_address(2);
default:
BUG();
}
#else
unsigned long entries[1];
struct stack_trace trace = {
.nr_entries = 0,
.entries = entries,
.max_entries = 1,
.skip = arg
};
save_stack_trace(&trace);
return entries[0];
#endif
}


#endif
8 changes: 4 additions & 4 deletions mm/kmsan/kmsan_instr.c
Expand Up @@ -398,8 +398,7 @@ void *__msan_memcpy(void *dst, const void *src, u64 n)
#if 0
if ((dst != src) && (!(((u64)dst + n <= (u64)src) || ((u64)src + n <= (u64)dst)))) {
kmsan_pr_err("==================================================================\n");
// TODO(glider): avoid __builtin_return_address(1).
kmsan_pr_err("WARNING: memcpy-param-overlap in %pS\n", __builtin_return_address(1));
kmsan_pr_err("WARNING: memcpy-param-overlap in %pS\n", kmsan_internal_return_address(1));
kmsan_pr_err("__msan_memcpy(%px, %px, %d)\n", dst, src, n);
dump_stack();
kmsan_pr_err("==================================================================\n");
Expand Down Expand Up @@ -558,7 +557,8 @@ inline void kmsan_set_origin_inline(u64 address, int size, u32 origin)
}


void __msan_poison_alloca(u64 address, u64 size, char *descr/*checked*/, u64 pc)
// TODO(glider): drop the last argument.
void __msan_poison_alloca(u64 address, u64 size, char *descr/*checked*/, u64 unused)
{
depot_stack_handle_t handle;
BUG_ON(size > PAGE_SIZE * 4);
Expand Down Expand Up @@ -600,7 +600,7 @@ void __msan_poison_alloca(u64 address, u64 size, char *descr/*checked*/, u64 pc)
entries[0] = KMSAN_ALLOCA_MAGIC_ORIGIN;
entries[1] = (u64)descr;
entries[2] = __builtin_return_address(0);
entries[3] = pc;
entries[3] = kmsan_internal_return_address(1);

ENTER_RUNTIME(irq_flags);
handle = depot_save_stack(&trace, GFP_ATOMIC);
Expand Down

0 comments on commit 27aa601

Please sign in to comment.