Re: [PATCH 1/8] kasan,x86: Frob kasan_report() in an exception

From: Peter Zijlstra
Date: Wed Mar 06 2019 - 08:14:01 EST


On Fri, Mar 01, 2019 at 04:23:05PM +0100, Peter Zijlstra wrote:

> But yes, I'll try some annotation, see what that looks like.

OK; that took a lot of time.. and a number of objtool bugs fixed but I
think I have something that I don't hate -- although it is not as solid
as I'd like it to be.


unmodified:

0000 0000000000000150 <__asan_load1>:
0000 150: 48 b8 ff ff ff ff ff movabs $0xffff7fffffffffff,%rax
0007 157: 7f ff ff
000a 15a: 48 8b 0c 24 mov (%rsp),%rcx
000e 15e: 48 39 c7 cmp %rax,%rdi
0011 161: 76 23 jbe 186 <__asan_load1+0x36>
0013 163: 48 b8 00 00 00 00 00 movabs $0xdffffc0000000000,%rax
001a 16a: fc ff df
001d 16d: 48 89 fa mov %rdi,%rdx
0020 170: 48 c1 ea 03 shr $0x3,%rdx
0024 174: 0f b6 04 02 movzbl (%rdx,%rax,1),%eax
0028 178: 84 c0 test %al,%al
002a 17a: 75 01 jne 17d <__asan_load1+0x2d>
002c 17c: c3 retq
002d 17d: 89 fa mov %edi,%edx
002f 17f: 83 e2 07 and $0x7,%edx
0032 182: 38 d0 cmp %dl,%al
0034 184: 7f f6 jg 17c <__asan_load1+0x2c>
0036 186: 31 d2 xor %edx,%edx
0038 188: be 01 00 00 00 mov $0x1,%esi
003d 18d: e9 00 00 00 00 jmpq 192 <__asan_load1+0x42>
003e 18e: R_X86_64_PLT32 kasan_report-0x4

exception:

0000 0000000000000150 <__asan_load1>:
0000 150: 48 b8 ff ff ff ff ff movabs $0xffff7fffffffffff,%rax
0007 157: 7f ff ff
000a 15a: 48 8b 0c 24 mov (%rsp),%rcx
000e 15e: 48 39 c7 cmp %rax,%rdi
0011 161: 76 23 jbe 186 <__asan_load1+0x36>
0013 163: 48 b8 00 00 00 00 00 movabs $0xdffffc0000000000,%rax
001a 16a: fc ff df
001d 16d: 48 89 fa mov %rdi,%rdx
0020 170: 48 c1 ea 03 shr $0x3,%rdx
0024 174: 0f b6 04 02 movzbl (%rdx,%rax,1),%eax
0028 178: 84 c0 test %al,%al
002a 17a: 75 01 jne 17d <__asan_load1+0x2d>
002c 17c: c3 retq
002d 17d: 89 fa mov %edi,%edx
002f 17f: 83 e2 07 and $0x7,%edx
0032 182: 38 d0 cmp %dl,%al
0034 184: 7f f6 jg 17c <__asan_load1+0x2c>
0036 186: 65 48 8b 04 25 00 00 mov %gs:0x0,%rax
003d 18d: 00 00
003b 18b: R_X86_64_32S current_task
003f 18f: 8b 80 68 08 00 00 mov 0x868(%rax),%eax
0045 195: 85 c0 test %eax,%eax
0047 197: 75 e3 jne 17c <__asan_load1+0x2c>
0049 199: be 01 00 00 00 mov $0x1,%esi
004e 19e: 31 d2 xor %edx,%edx
0050 1a0: 0f 0b ud2
0052 1a2: c3 retq

annotated:

0000 0000000000000150 <__asan_load1>:
0000 150: 48 b8 ff ff ff ff ff movabs $0xffff7fffffffffff,%rax
0007 157: 7f ff ff
000a 15a: 53 push %rbx
000b 15b: 48 8b 4c 24 08 mov 0x8(%rsp),%rcx
0010 160: 48 39 c7 cmp %rax,%rdi
0013 163: 76 24 jbe 189 <__asan_load1+0x39>
0015 165: 48 b8 00 00 00 00 00 movabs $0xdffffc0000000000,%rax
001c 16c: fc ff df
001f 16f: 48 89 fa mov %rdi,%rdx
0022 172: 48 c1 ea 03 shr $0x3,%rdx
0026 176: 0f b6 04 02 movzbl (%rdx,%rax,1),%eax
002a 17a: 84 c0 test %al,%al
002c 17c: 75 02 jne 180 <__asan_load1+0x30>
002e 17e: 5b pop %rbx
002f 17f: c3 retq
0030 180: 89 fa mov %edi,%edx
0032 182: 83 e2 07 and $0x7,%edx
0035 185: 38 d0 cmp %dl,%al
0037 187: 7f f5 jg 17e <__asan_load1+0x2e>
0039 189: 9c pushfq
003a 18a: 5b pop %rbx
003b 18b: 90 nop
003c 18c: 90 nop
003d 18d: 90 nop
003e 18e: 31 d2 xor %edx,%edx
0040 190: be 01 00 00 00 mov $0x1,%esi
0045 195: e8 00 00 00 00 callq 19a <__asan_load1+0x4a>
0046 196: R_X86_64_PLT32 __kasan_report-0x4
004a 19a: 53 push %rbx
004b 19b: 9d popfq
004c 19c: 5b pop %rbx
004d 19d: c3 retq


---
--- a/arch/x86/include/asm/kasan.h
+++ b/arch/x86/include/asm/kasan.h
@@ -28,6 +28,23 @@
#ifdef CONFIG_KASAN
void __init kasan_early_init(void);
void __init kasan_init(void);
+
+#include <asm/smap.h>
+
+extern void __kasan_report(unsigned long addr, size_t size, bool is_write, unsigned long ip);
+
+static __always_inline
+void kasan_report(unsigned long addr, size_t size, bool is_write, unsigned long ip)
+{
+ unsigned long flags;
+
+ flags = smap_save();
+ __kasan_report(addr, size, is_write, ip);
+ smap_restore(flags);
+
+}
+#define kasan_report kasan_report
+
#else
static inline void kasan_early_init(void) { }
static inline void kasan_init(void) { }
--- a/arch/x86/include/asm/smap.h
+++ b/arch/x86/include/asm/smap.h
@@ -46,6 +46,8 @@

#ifdef CONFIG_X86_SMAP

+#include <asm/irqflags.h>
+
static __always_inline void clac(void)
{
/* Note: a barrier is implicit in alternative() */
@@ -58,6 +60,18 @@ static __always_inline void stac(void)
alternative("", __stringify(__ASM_STAC), X86_FEATURE_SMAP);
}

+static __always_inline unsigned long smap_save(void)
+{
+ unsigned long flags = arch_local_save_flags();
+ clac();
+ return flags;
+}
+
+static __always_inline void smap_restore(unsigned long flags)
+{
+ arch_local_irq_restore(flags);
+}
+
/* These macros can be used in asm() statements */
#define ASM_CLAC \
ALTERNATIVE("", __stringify(__ASM_CLAC), X86_FEATURE_SMAP)
@@ -69,6 +83,9 @@ static __always_inline void stac(void)
static inline void clac(void) { }
static inline void stac(void) { }

+static inline unsigned long smap_save(void) { return 0; }
+static inline void smap_restore(unsigned long flags) { }
+
#define ASM_CLAC
#define ASM_STAC

--- a/include/linux/kasan.h
+++ b/include/linux/kasan.h
@@ -83,6 +83,8 @@ size_t kasan_metadata_size(struct kmem_c
bool kasan_save_enable_multi_shot(void);
void kasan_restore_multi_shot(bool enabled);

+void __kasan_report(unsigned long addr, size_t size, bool is_write, unsigned long ip);
+
#else /* CONFIG_KASAN */

static inline void kasan_unpoison_shadow(const void *address, size_t size) {}
@@ -153,8 +155,14 @@ static inline void kasan_remove_zero_sha
static inline void kasan_unpoison_slab(const void *ptr) { }
static inline size_t kasan_metadata_size(struct kmem_cache *cache) { return 0; }

+static inline void __kasan_report(unsigned long addr, size_t size, bool is_write, unsigned long ip) { }
+
#endif /* CONFIG_KASAN */

+#ifndef kasan_report
+#define kasan_report(addr, size, is_write, ip) __kasan_report(addr, size, is_write, ip)
+#endif
+
#ifdef CONFIG_KASAN_GENERIC

#define KASAN_SHADOW_INIT 0
@@ -177,9 +185,6 @@ void kasan_init_tags(void);

void *kasan_reset_tag(const void *addr);

-void kasan_report(unsigned long addr, size_t size,
- bool is_write, unsigned long ip);
-
#else /* CONFIG_KASAN_SW_TAGS */

static inline void kasan_init_tags(void) { }
--- a/mm/kasan/generic_report.c
+++ b/mm/kasan/generic_report.c
@@ -118,14 +118,14 @@ const char *get_bug_type(struct kasan_ac
#define DEFINE_ASAN_REPORT_LOAD(size) \
void __asan_report_load##size##_noabort(unsigned long addr) \
{ \
- kasan_report(addr, size, false, _RET_IP_); \
+ __kasan_report(addr, size, false, _RET_IP_); \
} \
EXPORT_SYMBOL(__asan_report_load##size##_noabort)

#define DEFINE_ASAN_REPORT_STORE(size) \
void __asan_report_store##size##_noabort(unsigned long addr) \
{ \
- kasan_report(addr, size, true, _RET_IP_); \
+ __kasan_report(addr, size, true, _RET_IP_); \
} \
EXPORT_SYMBOL(__asan_report_store##size##_noabort)

@@ -142,12 +142,12 @@ DEFINE_ASAN_REPORT_STORE(16);

void __asan_report_load_n_noabort(unsigned long addr, size_t size)
{
- kasan_report(addr, size, false, _RET_IP_);
+ __kasan_report(addr, size, false, _RET_IP_);
}
EXPORT_SYMBOL(__asan_report_load_n_noabort);

void __asan_report_store_n_noabort(unsigned long addr, size_t size)
{
- kasan_report(addr, size, true, _RET_IP_);
+ __kasan_report(addr, size, true, _RET_IP_);
}
EXPORT_SYMBOL(__asan_report_store_n_noabort);
--- a/mm/kasan/kasan.h
+++ b/mm/kasan/kasan.h
@@ -130,8 +130,6 @@ void check_memory_region(unsigned long a
void *find_first_bad_addr(void *addr, size_t size);
const char *get_bug_type(struct kasan_access_info *info);

-void kasan_report(unsigned long addr, size_t size,
- bool is_write, unsigned long ip);
void kasan_report_invalid_free(void *object, unsigned long ip);

#if defined(CONFIG_KASAN_GENERIC) && \
--- a/mm/kasan/report.c
+++ b/mm/kasan/report.c
@@ -281,7 +281,7 @@ void kasan_report_invalid_free(void *obj
end_report(&flags);
}

-void kasan_report(unsigned long addr, size_t size,
+void __kasan_report(unsigned long addr, size_t size,
bool is_write, unsigned long ip)
{
struct kasan_access_info info;
--- a/tools/objtool/arch.h
+++ b/tools/objtool/arch.h
@@ -43,6 +43,7 @@ enum op_dest_type {
OP_DEST_REG_INDIRECT,
OP_DEST_MEM,
OP_DEST_PUSH,
+ OP_DEST_PUSHF,
OP_DEST_LEAVE,
};

@@ -57,6 +58,7 @@ enum op_src_type {
OP_SRC_REG_INDIRECT,
OP_SRC_CONST,
OP_SRC_POP,
+ OP_SRC_POPF,
OP_SRC_ADD,
OP_SRC_AND,
};
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -357,13 +357,13 @@ int arch_decode_instruction(struct elf *
/* pushf */
*type = INSN_STACK;
op->src.type = OP_SRC_CONST;
- op->dest.type = OP_DEST_PUSH;
+ op->dest.type = OP_DEST_PUSHF;
break;

case 0x9d:
/* popf */
*type = INSN_STACK;
- op->src.type = OP_SRC_POP;
+ op->src.type = OP_SRC_POPF;
op->dest.type = OP_DEST_MEM;
break;

--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1359,11 +1359,11 @@ static int update_insn_state_regs(struct
return 0;

/* push */
- if (op->dest.type == OP_DEST_PUSH)
+ if (op->dest.type == OP_DEST_PUSH || op->dest.type == OP_DEST_PUSHF)
cfa->offset += 8;

/* pop */
- if (op->src.type == OP_SRC_POP)
+ if (op->src.type == OP_SRC_POP || op->src.type == OP_SRC_POPF)
cfa->offset -= 8;

/* add immediate to sp */
@@ -1620,6 +1620,7 @@ static int update_insn_state(struct inst
break;

case OP_SRC_POP:
+ case OP_SRC_POPF:
if (!state->drap && op->dest.type == OP_DEST_REG &&
op->dest.reg == cfa->base) {

@@ -1684,6 +1685,7 @@ static int update_insn_state(struct inst
break;

case OP_DEST_PUSH:
+ case OP_DEST_PUSHF:
state->stack_size += 8;
if (cfa->base == CFI_SP)
cfa->offset += 8;
@@ -1774,7 +1776,7 @@ static int update_insn_state(struct inst
break;

case OP_DEST_MEM:
- if (op->src.type != OP_SRC_POP) {
+ if (op->src.type != OP_SRC_POP && op->src.type != OP_SRC_POPF) {
WARN_FUNC("unknown stack-related memory operation",
insn->sec, insn->offset);
return -1;
@@ -2071,6 +2073,16 @@ static int validate_branch(struct objtoo
if (update_insn_state(insn, &state))
return 1;

+ if (insn->stack_op.dest.type == OP_DEST_PUSHF) {
+ if (state.uaccess)
+ state.uaccess_stack++;
+ }
+
+ if (insn->stack_op.src.type == OP_SRC_POPF) {
+ if (state.uaccess_stack && !--state.uaccess_stack)
+ state.uaccess = func_uaccess_safe(func);
+ }
+
break;

case INSN_STAC:
@@ -2088,7 +2100,7 @@ static int validate_branch(struct objtoo
return 1;
}

- if (func_uaccess_safe(func)) {
+ if (func_uaccess_safe(func) && !state.uaccess_stack) {
WARN_FUNC("UACCESS-safe disables UACCESS", sec, insn->offset);
return 1;
}
--- a/tools/objtool/check.h
+++ b/tools/objtool/check.h
@@ -32,6 +32,7 @@ struct insn_state {
unsigned char type;
bool bp_scratch;
bool drap, end, uaccess;
+ int uaccess_stack;
int drap_reg, drap_offset;
struct cfi_reg vals[CFI_NUM_REGS];
};