FYI, we noticed the following commit (built with gcc-4.9):
commit: 51bad67ffbce0aaa44579f84ef5d05597054ec6a ("x86/asm: Pad assembly functions with INT3 instructions")
https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git x86/pti
in testcase: boot
on test machine: qemu-system-i386 -enable-kvm -m 256M
caused below changes (please refer to attached dmesg/kmsg for entire log/backtrace):
+-------------------------------------------+------------+------------+
| | e0f6d1a526 | 51bad67ffb |
+-------------------------------------------+------------+------------+
| boot_successes | 0 | 0 |
| boot_failures | 24 | 24 |
| WARNING:at_kernel/kthread.c:#kthread_park | 24 | 24 |
| EIP:kthread_park | 24 | 24 |
| int3:#[##] | 0 | 24 |
| EIP:ret_from_intr | 0 | 24 |
| Kernel_panic-not_syncing:Fatal_exception | 0 | 24 |
+-------------------------------------------+------------+------------+
[ 11.174401] Write protecting the kernel text: 7192k
[ 11.174882] Write protecting the kernel read-only data: 2796k
[ 11.175437] rodata_test: all tests were successful
[ 11.176131] random: get_random_u32 called from arch_rnd+0x1e/0x40 with crng_init=0
[ 11.176836] random: get_random_u32 called from load_elf_binary+0x32d/0x12c4 with crng_init=0
[ 11.177719] int3: 0000 [#1] PREEMPT DEBUG_PAGEALLOC
[ 11.178183] Modules linked in:
[ 11.178501] CPU: 0 PID: 1 Comm: init Tainted: G W 4.17.0-rc3-00048-g51bad67 #2
[ 11.179288] EIP: ret_from_intr+0xd/0x14
[ 11.179647] EFLAGS: 00000046 CPU: 0
[ 11.179975] EAX: 00000003 EBX: 00000000 ECX: 00000000 EDX: 00000000
[ 11.180553] ESI: 00000000 EDI: 79048b50 EBP: 0603bfb4 ESP: 8603bfb4
[ 11.181188] DS: 007b ES: 007b FS: 0000 GS: 00e0 SS: 0068
[ 11.181675] CR0: 80050033 CR2: 6ffce1bf CR3: 0b2c9000 CR4: 00000690
[ 11.182269] Call Trace:
[ 11.182501] Code: 00 00 00 eb e6 cc cc cc cc cc cc cc cc cc cc cc cc cc fa 8d b6 00 00 00 00 e8 5d e8 8f ff 8b 44 24 34 83 e0 03 83 f8 03 72 28 cc <cc> cc cc cc cc cc cc fa 8d b6 00 00 00 00 e8 3d e8 8f ff 89 e0
[ 11.184326] EIP: ret_from_intr+0xd/0x14 SS:ESP: 0068:8603bfb4
[ 11.184848] ---[ end trace 031a6bca415900de ]---
[ 11.185302] Kernel panic - not syncing: Fatal exception
[ 11.185782] Kernel Offset: disabled
Elapsed time: 20
#!/bin/bash
To reproduce:
git clone https://github.com/intel/lkp-tests.git
cd lkp-tests
bin/lkp qemu -k <bzImage> job-script # job-script is attached in this email
Thanks,
Xiaolong
On Tue, May 15, 2018 at 04:00:33PM +0800, kernel test robot wrote:
> [ 11.182501] Code: 00 00 00 eb e6 cc cc cc cc cc cc cc cc cc cc cc cc cc fa 8d b6 00 00 00 00 e8 5d e8 8f ff 8b 44 24 34 83 e0 03 83 f8 03 72 28 cc <cc> cc cc cc cc cc cc fa 8d b6 00 00 00 00 e8 3d e8 8f ff 89 e0
> [ 11.184326] EIP: ret_from_intr+0xd/0x14 SS:ESP: 0068:8603bfb4
INT3 slipped through M586 => X86_ALIGNMENT_16 :-\
On Tue, May 15, 2018 at 09:25:53PM +0000, Anvin, H Peter wrote:
> Why is that a problem?
> Code: 00 00 00 eb e6 cc cc cc cc cc cc cc cc cc cc cc cc cc fa 8d b6 00 00 00 00 e8 5d e8 8f ff 8b 44 24 34 83 e0 03 83 f8 03 72 28 cc <cc> cc cc cc cc cc cc fa 8d b6 00 00 00 00 e8 3d e8 8f ff 89 e0
>
> EIP: ret_from_intr+0xd/0x14 SS:ESP: 0068:8603bfb4
>
> INT3 slipped through M586 => X86_ALIGNMENT_16 :-\
I could make the patch x86_64 only, but!
It crashed into the middle of the padding.
796ef8fc <ret_from_intr>:
796ef8fc: 8b 44 24 34 mov eax,DWORD PTR [esp+0x34]
796ef900: 83 e0 03 and eax,0x3
796ef903: 83 f8 03 cmp eax,0x3
796ef906: 72 28 jb 796ef930 <resume_kernel>
796ef908: cc int3
796ef909: cc <========> int3
796ef90a: cc int3
796ef90b: cc int3
796ef90c: cc int3
796ef90d: cc int3
796ef90e: cc int3
796ef90f: cc int3
On Wed, May 16, 2018 at 12:43:37AM +0300, Alexey Dobriyan wrote:
> On Tue, May 15, 2018 at 09:25:53PM +0000, Anvin, H Peter wrote:
> > Why is that a problem?
> > Code: 00 00 00 eb e6 cc cc cc cc cc cc cc cc cc cc cc cc cc fa 8d b6 00 00 00 00 e8 5d e8 8f ff 8b 44 24 34 83 e0 03 83 f8 03 72 28 cc <cc> cc cc cc cc cc cc fa 8d b6 00 00 00 00 e8 3d e8 8f ff 89 e0
> >
> > EIP: ret_from_intr+0xd/0x14 SS:ESP: 0068:8603bfb4
> >
> > INT3 slipped through M586 => X86_ALIGNMENT_16 :-\
>
> I could make the patch x86_64 only, but!
>
> It crashed into the middle of the padding.
>
> 796ef8fc <ret_from_intr>:
> 796ef8fc: 8b 44 24 34 mov eax,DWORD PTR [esp+0x34]
> 796ef900: 83 e0 03 and eax,0x3
> 796ef903: 83 f8 03 cmp eax,0x3
> 796ef906: 72 28 jb 796ef930 <resume_kernel>
> 796ef908: cc int3
> 796ef909: cc <========> int3
> 796ef90a: cc int3
> 796ef90b: cc int3
> 796ef90c: cc int3
> 796ef90d: cc int3
> 796ef90e: cc int3
> 796ef90f: cc int3
The padding isn't needed there, and the resume_userspace symbol is never
used, so wouldn't this fix it?
diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
index bef8e2b202a8..9e56243c984c 100644
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -338,7 +338,6 @@ ret_from_intr:
cmpl $USER_RPL, %eax
jb resume_kernel # not returning to v8086 or userspace
-ENTRY(resume_userspace)
DISABLE_INTERRUPTS(CLBR_ANY)
TRACE_IRQS_OFF
movl %esp, %eax
On Wed, 16 May 2018, Alexey Dobriyan wrote:
> On Tue, May 15, 2018 at 09:25:53PM +0000, Anvin, H Peter wrote:
> > Why is that a problem?
> > Code: 00 00 00 eb e6 cc cc cc cc cc cc cc cc cc cc cc cc cc fa 8d b6 00 00 00 00 e8 5d e8 8f ff 8b 44 24 34 83 e0 03 83 f8 03 72 28 cc <cc> cc cc cc cc cc cc fa 8d b6 00 00 00 00 e8 3d e8 8f ff 89 e0
> >
> > EIP: ret_from_intr+0xd/0x14 SS:ESP: 0068:8603bfb4
> >
> > INT3 slipped through M586 => X86_ALIGNMENT_16 :-\
>
> I could make the patch x86_64 only, but!
>
> It crashed into the middle of the padding.
>
> 796ef8fc <ret_from_intr>:
> 796ef8fc: 8b 44 24 34 mov eax,DWORD PTR [esp+0x34]
> 796ef900: 83 e0 03 and eax,0x3
> 796ef903: 83 f8 03 cmp eax,0x3
> 796ef906: 72 28 jb 796ef930 <resume_kernel>
> 796ef908: cc int3
> 796ef909: cc <========> int3
EIP points to the second int3 because that's where a simple return would
return to. do_trap() does not handle int3 special.
From a quick check that seems to be the only code sequence which has:
jcc lbl
ENTRY(foo)
and ENTRY() aligns to a 16 byte boundary. So the simple fix for that is
below. We need objtool support to detect such places ...
Thanks,
tglx
8<-------------------
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -338,6 +338,11 @@ END(ret_from_fork)
cmpl $USER_RPL, %eax
jb resume_kernel # not returning to v8086 or userspace
+ /*
+ * Jump over the alignment padding which is filled with int3 instructions
+ */
+ jmp resume_userspace
+
ENTRY(resume_userspace)
DISABLE_INTERRUPTS(CLBR_ANY)
TRACE_IRQS_OFF
On Tue, 15 May 2018, Josh Poimboeuf wrote:
> On Wed, May 16, 2018 at 12:43:37AM +0300, Alexey Dobriyan wrote:
> > On Tue, May 15, 2018 at 09:25:53PM +0000, Anvin, H Peter wrote:
> > > Why is that a problem?
> > > Code: 00 00 00 eb e6 cc cc cc cc cc cc cc cc cc cc cc cc cc fa 8d b6 00 00 00 00 e8 5d e8 8f ff 8b 44 24 34 83 e0 03 83 f8 03 72 28 cc <cc> cc cc cc cc cc cc fa 8d b6 00 00 00 00 e8 3d e8 8f ff 89 e0
> > >
> > > EIP: ret_from_intr+0xd/0x14 SS:ESP: 0068:8603bfb4
> > >
> > > INT3 slipped through M586 => X86_ALIGNMENT_16 :-\
> >
> > I could make the patch x86_64 only, but!
> >
> > It crashed into the middle of the padding.
> >
> > 796ef8fc <ret_from_intr>:
> > 796ef8fc: 8b 44 24 34 mov eax,DWORD PTR [esp+0x34]
> > 796ef900: 83 e0 03 and eax,0x3
> > 796ef903: 83 f8 03 cmp eax,0x3
> > 796ef906: 72 28 jb 796ef930 <resume_kernel>
> > 796ef908: cc int3
> > 796ef909: cc <========> int3
> > 796ef90a: cc int3
> > 796ef90b: cc int3
> > 796ef90c: cc int3
> > 796ef90d: cc int3
> > 796ef90e: cc int3
> > 796ef90f: cc int3
>
> The padding isn't needed there, and the resume_userspace symbol is never
> used, so wouldn't this fix it?
Gack. Right you are. I assumed that the ENTRY() is used without checking ....
>
> diff --git a/arch/x86/entry/entry_32.S b/arch/x86/entry/entry_32.S
> index bef8e2b202a8..9e56243c984c 100644
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -338,7 +338,6 @@ ret_from_intr:
> cmpl $USER_RPL, %eax
> jb resume_kernel # not returning to v8086 or userspace
>
> -ENTRY(resume_userspace)
> DISABLE_INTERRUPTS(CLBR_ANY)
> TRACE_IRQS_OFF
> movl %esp, %eax
>
On Tue, May 15, 2018 at 2:43 PM Alexey Dobriyan <[email protected]> wrote:
> It crashed into the middle of the padding.
No, the beginning of the padding. "int3" will push the return address on
the stack, so when it points to the second 'int3' instruction, it's because
the first one triggered.
Linus
On Tue, May 15, 2018 at 3:22 PM Josh Poimboeuf <[email protected]> wrote:
> The padding isn't needed there, and the resume_userspace symbol is never
> used, so wouldn't this fix it?
This looks like the correct fix for this case, but are we sure there aren't
other cases where we have this same "fall through to an ENTRY" case?
Because we've definitely had that kind of code before too - sometimes
simply because we want profiles and oopses to show which "part" of the asm
we're faulting in (that could be the case here too).
Linus
On Tue, May 15, 2018 at 3:25 PM Thomas Gleixner <[email protected]> wrote:
> On Wed, 16 May 2018, Alexey Dobriyan wrote:
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -338,6 +338,11 @@ END(ret_from_fork)
> cmpl $USER_RPL, %eax
> jb resume_kernel # not returning to v8086
or userspace
> + /*
> + * Jump over the alignment padding which is filled with int3
instructions
> + */
> + jmp resume_userspace
> +
> ENTRY(resume_userspace)
The problem is already solved, but NAK to this concept. If we're going to
make ENTRY supply int3 alignment, then we need to avoid using ENTRY if we
fall through. Jumping over the alignment is just silly. The macro we
should use is GLOBAL.
On Tue, May 15, 2018 at 03:28:19PM -0700, Linus Torvalds wrote:
> On Tue, May 15, 2018 at 3:22 PM Josh Poimboeuf <[email protected]> wrote:
>
> > The padding isn't needed there, and the resume_userspace symbol is never
> > used, so wouldn't this fix it?
>
> This looks like the correct fix for this case, but are we sure there aren't
> other cases where we have this same "fall through to an ENTRY" case?
>
> Because we've definitely had that kind of code before too - sometimes
> simply because we want profiles and oopses to show which "part" of the asm
> we're faulting in (that could be the case here too).
Glancing through the 32-bit and 64-bit entry code, I didn't see any more
cases. At least it will fail loudly if any such cases do still exist.
--
Josh
On Tue, May 15, 2018 at 03:27:34PM -0700, Linus Torvalds wrote:
> On Tue, May 15, 2018 at 2:43 PM Alexey Dobriyan <[email protected]> wrote:
>
> > It crashed into the middle of the padding.
>
> No, the beginning of the padding. "int3" will push the return address on
> the stack, so when it points to the second 'int3' instruction, it's because
> the first one triggered.
Sending i386 patches should not be done that late...
I checked 32-bit asm as well.
On Tue, May 15, 2018 at 3:43 PM Josh Poimboeuf <[email protected]> wrote:
> Glancing through the 32-bit and 64-bit entry code, I didn't see any more
> cases. At least it will fail loudly if any such cases do still exist.
\Will it? Do we have objtool checks for it now?
Because without static checks, there could be things hiding that just don't
happen normally (think compat code etc that for most people is just dead
code).
Linus
Use INT3 instead of NOP. All that padding between functions is
an illegal area, no legitimate code should jump into it.
I've checked x86_64 allyesconfig disassembly, all changes looks sane:
INT3 is only used after RET or unconditional JMP.
On i386:
* promote ret_from_exception into ENTRY as it has corresponding END,
* demote "resume_userspace" -- unused,
* delete ALIGN directive in page_fault. It is leftover from x86 assembly
cleanups.
commit d211af055d0c12dc3416c2886e6fbdc6eb74a381
i386: get rid of the use of KPROBE_ENTRY / KPROBE_END
has ALIGN directive before branch target which makes sense.
All the code after ALIGN disappeared later.
Signed-off-by: Alexey Dobriyan <[email protected]>
---
arch/x86/entry/entry_32.S | 6 +-----
arch/x86/include/asm/linkage.h | 2 +-
2 files changed, 2 insertions(+), 6 deletions(-)
--- a/arch/x86/entry/entry_32.S
+++ b/arch/x86/entry/entry_32.S
@@ -320,8 +320,7 @@ END(ret_from_fork)
*/
# userspace resumption stub bypassing syscall exit tracing
- ALIGN
-ret_from_exception:
+ENTRY(ret_from_exception)
preempt_stop(CLBR_ANY)
ret_from_intr:
#ifdef CONFIG_VM86
@@ -337,8 +336,6 @@ ret_from_intr:
#endif
cmpl $USER_RPL, %eax
jb resume_kernel # not returning to v8086 or userspace
-
-ENTRY(resume_userspace)
DISABLE_INTERRUPTS(CLBR_ANY)
TRACE_IRQS_OFF
movl %esp, %eax
@@ -910,7 +907,6 @@ BUILD_INTERRUPT3(hv_stimer0_callback_vector, HYPERV_STIMER0_VECTOR,
ENTRY(page_fault)
ASM_CLAC
pushl $do_page_fault
- ALIGN
jmp common_exception
END(page_fault)
--- a/arch/x86/include/asm/linkage.h
+++ b/arch/x86/include/asm/linkage.h
@@ -18,7 +18,7 @@
name:
#if defined(CONFIG_X86_64) || defined(CONFIG_X86_ALIGNMENT_16)
-#define __ALIGN .p2align 4, 0x90
+#define __ALIGN .p2align 4, 0xCC
#define __ALIGN_STR __stringify(__ALIGN)
#endif
Side note: doing some grepping, I find some other sequences that are a bit
scary, like this:
arch/x86/kernel/acpi/wakeup_32.S-.data
arch/x86/kernel/acpi/wakeup_32.S-ALIGN
arch/x86/kernel/acpi/wakeup_32.S:ENTRY(saved_magic) .long 0
arch/x86/kernel/acpi/wakeup_32.S:ENTRY(saved_eip) .long 0
so apparently people are using ENTRY() for data too (the same pattern
exists in wakeup_64.S).
So we end up having those odd 0x90 bytes (now 0xcc) in the data section as
"padding" between those two values. Crazy.
Not an actual problem, but it does show that people seem to be mis-using
ENTRY().
There that padding is actually entirely wrong. We do *not* want to pad
between those "saved_xyz" fields. I think those should use GLOBAL too.
Or lookie here:
arch/x86/kernel/head_32.S-# boot GDT descriptor (later on used by CPU#0):
arch/x86/kernel/head_32.S- .word 0 # 32 bit
align gdt_desc.address
arch/x86/kernel/head_32.S:ENTRY(early_gdt_descr)
arch/x86/kernel/head_32.S- .word GDT_ENTRIES*8-1
arch/x86/kernel/head_32.S- .long gdt_page /*
Overwritten for secondary CPUs */
where ENTRY() will actually screw up how the code says it wants to align
the address, but now it doesn't.
I wonder if there's some way to add a test for "ENTRY only works in a code
section"?
My grep was just me doing a visual scan, I might have missed something.
Linus
On Tue, May 15, 2018 at 3:58 PM Alexey Dobriyan <[email protected]> wrote:
> jb resume_kernel # not returning to v8086
or userspace
> -
> -ENTRY(resume_userspace)
I went back in history, and we used to have multiple "jmp resume_userspace"
including even the vm86.c code (in a different file) doing that, so there
*used* to be a reason for this being a global symbol (and yes, we had a
fall-through to that global symbol even in those days).
That said, I also look at "resume_kernel". There's no reason to make that
globally visible afaik. It looks local to entry_32.S, and should look like
restore_all rather than resume_userspace.
So we probably should remove the ENTRY() for that too, and just make it be
ALIGN
resume_kernel:
instead?
I guess the reason for removing that other ENTRY() is different from this
case, though..
Linus
On Tue, May 15, 2018 at 04:05:33PM -0700, Linus Torvalds wrote:
> I wonder if there's some way to add a test for "ENTRY only works in a code
> section"?
I suppose we could add a discardable annotation to the ENTRY macro and
have objtool validate that it's in a text section. I'm not sure whether
it's worth it, but I could do it if you think it's a good idea.
Below is a tentative objtool patch which catches asm code falling
through to INT3 padding, though objtool is 64-bit only so there won't be
any 32-bit coverage. It found zero hits on my config. I'll clean it up
and submit it tomorrow-ish.
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 8344dd2f310a..3ed8cec6e765 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -285,11 +285,9 @@ ENTRY(early_idt_handler_array)
.endif
pushq $i # 72(%rsp) Vector number
jmp early_idt_handler_common
- UNWIND_HINT_IRET_REGS
i = i + 1
.fill early_idt_handler_array + i*EARLY_IDT_HANDLER_SIZE - ., 1, 0xcc
.endr
- UNWIND_HINT_IRET_REGS offset=16
END(early_idt_handler_array)
early_idt_handler_common:
diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h
index b0d7dc3d71b5..6eb058a8ac00 100644
--- a/tools/objtool/arch.h
+++ b/tools/objtool/arch.h
@@ -33,7 +33,8 @@
#define INSN_STACK 8
#define INSN_BUG 9
#define INSN_NOP 10
-#define INSN_OTHER 11
+#define INSN_PADDING 11
+#define INSN_OTHER 12
#define INSN_LAST INSN_OTHER
enum op_dest_type {
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 7e86a743f851..82b41bb93c02 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -421,7 +421,7 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
case 0xcc:
/* int3: used for asm function padding by the __ALIGN macro */
- *type = INSN_NOP;
+ *type = INSN_PADDING;
break;
case 0xe3:
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index f4bbce838433..8a83a0d1693a 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1936,6 +1936,14 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
}
return 0;
+ case INSN_PADDING:
+ if (!func) {
+ WARN_FUNC("code falls through to INT3 padding",
+ insn->sec, insn->offset);
+ return 1;
+ }
+ break;
+
case INSN_STACK:
if (update_insn_state(insn, &state))
return 1;
@@ -2032,7 +2040,8 @@ static bool ignore_unreachable_insn(struct instruction *insn)
{
int i;
- if (insn->ignore || insn->type == INSN_NOP)
+ if (insn->ignore || insn->type == INSN_NOP ||
+ insn->type == INSN_PADDING)
return true;
/*
With the following commit:
51bad67ffbce ("x86/asm: Pad assembly functions with INT3 instructions")
... asm function alignments are padded with INT3, so it's no longer safe
to fall through to an aligned function. Make sure we catch any such
cases with objtool.
Note this only adds checking for 64-bit, since objtool doesn't support
x86-32.
Suggested-by: Thomas Gleixner <[email protected]>
Suggested-by: Linus Torvalds <[email protected]>
Signed-off-by: Josh Poimboeuf <[email protected]>
---
arch/x86/kernel/head_64.S | 2 --
tools/objtool/arch.h | 3 ++-
tools/objtool/arch/x86/decode.c | 2 +-
tools/objtool/check.c | 11 ++++++++++-
4 files changed, 13 insertions(+), 5 deletions(-)
diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
index 8344dd2f310a..3ed8cec6e765 100644
--- a/arch/x86/kernel/head_64.S
+++ b/arch/x86/kernel/head_64.S
@@ -285,11 +285,9 @@ ENTRY(early_idt_handler_array)
.endif
pushq $i # 72(%rsp) Vector number
jmp early_idt_handler_common
- UNWIND_HINT_IRET_REGS
i = i + 1
.fill early_idt_handler_array + i*EARLY_IDT_HANDLER_SIZE - ., 1, 0xcc
.endr
- UNWIND_HINT_IRET_REGS offset=16
END(early_idt_handler_array)
early_idt_handler_common:
diff --git a/tools/objtool/arch.h b/tools/objtool/arch.h
index b0d7dc3d71b5..6eb058a8ac00 100644
--- a/tools/objtool/arch.h
+++ b/tools/objtool/arch.h
@@ -33,7 +33,8 @@
#define INSN_STACK 8
#define INSN_BUG 9
#define INSN_NOP 10
-#define INSN_OTHER 11
+#define INSN_PADDING 11
+#define INSN_OTHER 12
#define INSN_LAST INSN_OTHER
enum op_dest_type {
diff --git a/tools/objtool/arch/x86/decode.c b/tools/objtool/arch/x86/decode.c
index 7e86a743f851..82b41bb93c02 100644
--- a/tools/objtool/arch/x86/decode.c
+++ b/tools/objtool/arch/x86/decode.c
@@ -421,7 +421,7 @@ int arch_decode_instruction(struct elf *elf, struct section *sec,
case 0xcc:
/* int3: used for asm function padding by the __ALIGN macro */
- *type = INSN_NOP;
+ *type = INSN_PADDING;
break;
case 0xe3:
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index f4bbce838433..4d387448519f 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -1936,6 +1936,14 @@ static int validate_branch(struct objtool_file *file, struct instruction *first,
}
return 0;
+ case INSN_PADDING:
+ if (!file->c_file) {
+ WARN_FUNC("code falls through to INT3 padding",
+ insn->sec, insn->offset);
+ return 1;
+ }
+ break;
+
case INSN_STACK:
if (update_insn_state(insn, &state))
return 1;
@@ -2032,7 +2040,8 @@ static bool ignore_unreachable_insn(struct instruction *insn)
{
int i;
- if (insn->ignore || insn->type == INSN_NOP)
+ if (insn->ignore || insn->type == INSN_NOP ||
+ insn->type == INSN_PADDING)
return true;
/*
--
2.17.0
On Thu, May 17, 2018 at 08:49:34AM -0500, Josh Poimboeuf wrote:
> With the following commit:
>
> 51bad67ffbce ("x86/asm: Pad assembly functions with INT3 instructions")
>
> ... asm function alignments are padded with INT3, so it's no longer safe
> to fall through to an aligned function. Make sure we catch any such
> cases with objtool.
>
> Note this only adds checking for 64-bit, since objtool doesn't support
> x86-32.
>
> Suggested-by: Thomas Gleixner <[email protected]>
> Suggested-by: Linus Torvalds <[email protected]>
> Signed-off-by: Josh Poimboeuf <[email protected]>
> ---
> arch/x86/kernel/head_64.S | 2 --
> tools/objtool/arch.h | 3 ++-
> tools/objtool/arch/x86/decode.c | 2 +-
> tools/objtool/check.c | 11 ++++++++++-
> 4 files changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
> index 8344dd2f310a..3ed8cec6e765 100644
> --- a/arch/x86/kernel/head_64.S
> +++ b/arch/x86/kernel/head_64.S
> @@ -285,11 +285,9 @@ ENTRY(early_idt_handler_array)
> .endif
> pushq $i # 72(%rsp) Vector number
> jmp early_idt_handler_common
> - UNWIND_HINT_IRET_REGS
> i = i + 1
> .fill early_idt_handler_array + i*EARLY_IDT_HANDLER_SIZE - ., 1, 0xcc
> .endr
> - UNWIND_HINT_IRET_REGS offset=16
> END(early_idt_handler_array)
>
> early_idt_handler_common:
As noted on IRC; I got slightly confused what this was about.
Other than that:
Acked-by: Peter Zijlstra (Intel) <[email protected]>
* Linus Torvalds <[email protected]> wrote:
> On Tue, May 15, 2018 at 3:43 PM Josh Poimboeuf <[email protected]> wrote:
>
> > Glancing through the 32-bit and 64-bit entry code, I didn't see any more
> > cases. At least it will fail loudly if any such cases do still exist.
>
> \Will it? Do we have objtool checks for it now?
>
> Because without static checks, there could be things hiding that just don't
> happen normally (think compat code etc that for most people is just dead
> code).
And it's not just about infrequent uses, it's also about hard to debug crashes
like in suspend/resume code, where people can only report "machine is dead".
So I'm not sure this side effect of turning padding into crashes is good for
overall robustness, without static analysis finding such bugs.
Thanks,
Ingo
* Josh Poimboeuf <[email protected]> wrote:
> With the following commit:
>
> 51bad67ffbce ("x86/asm: Pad assembly functions with INT3 instructions")
>
> ... asm function alignments are padded with INT3, so it's no longer safe
> to fall through to an aligned function. Make sure we catch any such
> cases with objtool.
>
> Note this only adds checking for 64-bit, since objtool doesn't support
> x86-32.
>
> Suggested-by: Thomas Gleixner <[email protected]>
> Suggested-by: Linus Torvalds <[email protected]>
> Signed-off-by: Josh Poimboeuf <[email protected]>
> ---
> arch/x86/kernel/head_64.S | 2 --
> tools/objtool/arch.h | 3 ++-
> tools/objtool/arch/x86/decode.c | 2 +-
> tools/objtool/check.c | 11 ++++++++++-
> 4 files changed, 13 insertions(+), 5 deletions(-)
Ok, this is cool, it addresses the robustness problem that INT3 padding introduced
very nicely.
The concept of built-in kernel tooling working at the machine code level is just
so powerful - we should have added our own KCC compiler 20 years ago.
Thanks,
Ingo
* Peter Zijlstra <[email protected]> wrote:
> On Thu, May 17, 2018 at 08:49:34AM -0500, Josh Poimboeuf wrote:
> > With the following commit:
> >
> > 51bad67ffbce ("x86/asm: Pad assembly functions with INT3 instructions")
> >
> > ... asm function alignments are padded with INT3, so it's no longer safe
> > to fall through to an aligned function. Make sure we catch any such
> > cases with objtool.
> >
> > Note this only adds checking for 64-bit, since objtool doesn't support
> > x86-32.
> >
> > Suggested-by: Thomas Gleixner <[email protected]>
> > Suggested-by: Linus Torvalds <[email protected]>
> > Signed-off-by: Josh Poimboeuf <[email protected]>
> > ---
> > arch/x86/kernel/head_64.S | 2 --
> > tools/objtool/arch.h | 3 ++-
> > tools/objtool/arch/x86/decode.c | 2 +-
> > tools/objtool/check.c | 11 ++++++++++-
> > 4 files changed, 13 insertions(+), 5 deletions(-)
> >
> > diff --git a/arch/x86/kernel/head_64.S b/arch/x86/kernel/head_64.S
> > index 8344dd2f310a..3ed8cec6e765 100644
> > --- a/arch/x86/kernel/head_64.S
> > +++ b/arch/x86/kernel/head_64.S
> > @@ -285,11 +285,9 @@ ENTRY(early_idt_handler_array)
> > .endif
> > pushq $i # 72(%rsp) Vector number
> > jmp early_idt_handler_common
> > - UNWIND_HINT_IRET_REGS
> > i = i + 1
> > .fill early_idt_handler_array + i*EARLY_IDT_HANDLER_SIZE - ., 1, 0xcc
> > .endr
> > - UNWIND_HINT_IRET_REGS offset=16
> > END(early_idt_handler_array)
> >
> > early_idt_handler_common:
>
> As noted on IRC; I got slightly confused what this was about.
>
> Other than that:
>
> Acked-by: Peter Zijlstra (Intel) <[email protected]>
And after talking to you on IRC I added this paragraph to the changelog:
Also remove incorrect and unnecessary unwinder hints from head_64.S
which caused false positives in the new detection code.
Thanks,
Ingo
* Ingo Molnar <[email protected]> wrote:
>
> * Josh Poimboeuf <[email protected]> wrote:
>
> > With the following commit:
> >
> > 51bad67ffbce ("x86/asm: Pad assembly functions with INT3 instructions")
> >
> > ... asm function alignments are padded with INT3, so it's no longer safe
> > to fall through to an aligned function. Make sure we catch any such
> > cases with objtool.
> >
> > Note this only adds checking for 64-bit, since objtool doesn't support
> > x86-32.
> >
> > Suggested-by: Thomas Gleixner <[email protected]>
> > Suggested-by: Linus Torvalds <[email protected]>
> > Signed-off-by: Josh Poimboeuf <[email protected]>
> > ---
> > arch/x86/kernel/head_64.S | 2 --
> > tools/objtool/arch.h | 3 ++-
> > tools/objtool/arch/x86/decode.c | 2 +-
> > tools/objtool/check.c | 11 ++++++++++-
> > 4 files changed, 13 insertions(+), 5 deletions(-)
>
> Ok, this is cool, it addresses the robustness problem that INT3 padding introduced
> very nicely.
>
> The concept of built-in kernel tooling working at the machine code level is just
> so powerful - we should have added our own KCC compiler 20 years ago.
Hm, so a problem is that if we change the padding on 32-bit as well we won't have
this detection there, because objtool doesn't work on 32-bit.
Thanks,
Ingo
On 05/18/18 00:18, Ingo Molnar wrote:
>
> Ok, this is cool, it addresses the robustness problem that INT3 padding introduced
> very nicely.
>
> The concept of built-in kernel tooling working at the machine code level is just
> so powerful - we should have added our own KCC compiler 20 years ago.
>
How many times has this been suggested? ;)
-hpa
* Alexey Dobriyan <[email protected]> wrote:
> Use INT3 instead of NOP. All that padding between functions is
> an illegal area, no legitimate code should jump into it.
>
> I've checked x86_64 allyesconfig disassembly, all changes looks sane:
> INT3 is only used after RET or unconditional JMP.
>
> On i386:
> * promote ret_from_exception into ENTRY as it has corresponding END,
> * demote "resume_userspace" -- unused,
> * delete ALIGN directive in page_fault. It is leftover from x86 assembly
> cleanups.
>
> commit d211af055d0c12dc3416c2886e6fbdc6eb74a381
> i386: get rid of the use of KPROBE_ENTRY / KPROBE_END
>
> has ALIGN directive before branch target which makes sense.
> All the code after ALIGN disappeared later.
>
> Signed-off-by: Alexey Dobriyan <[email protected]>
> ---
>
> arch/x86/entry/entry_32.S | 6 +-----
> arch/x86/include/asm/linkage.h | 2 +-
> 2 files changed, 2 insertions(+), 6 deletions(-)
>
> --- a/arch/x86/entry/entry_32.S
> +++ b/arch/x86/entry/entry_32.S
> @@ -320,8 +320,7 @@ END(ret_from_fork)
> */
>
> # userspace resumption stub bypassing syscall exit tracing
> - ALIGN
> -ret_from_exception:
> +ENTRY(ret_from_exception)
> preempt_stop(CLBR_ANY)
> ret_from_intr:
> #ifdef CONFIG_VM86
> @@ -337,8 +336,6 @@ ret_from_intr:
> #endif
> cmpl $USER_RPL, %eax
> jb resume_kernel # not returning to v8086 or userspace
> -
> -ENTRY(resume_userspace)
> DISABLE_INTERRUPTS(CLBR_ANY)
> TRACE_IRQS_OFF
> movl %esp, %eax
> @@ -910,7 +907,6 @@ BUILD_INTERRUPT3(hv_stimer0_callback_vector, HYPERV_STIMER0_VECTOR,
> ENTRY(page_fault)
> ASM_CLAC
> pushl $do_page_fault
> - ALIGN
> jmp common_exception
> END(page_fault)
>
> --- a/arch/x86/include/asm/linkage.h
> +++ b/arch/x86/include/asm/linkage.h
> @@ -18,7 +18,7 @@
> name:
>
> #if defined(CONFIG_X86_64) || defined(CONFIG_X86_ALIGNMENT_16)
> -#define __ALIGN .p2align 4, 0x90
> +#define __ALIGN .p2align 4, 0xCC
> #define __ALIGN_STR __stringify(__ALIGN)
> #endif
So the question is, without objtool support, how will we find INT3-padding related
crash bugs on 32-bit kernels?
Thanks,
Ingo
On Fri, May 18, 2018 at 09:36:44AM +0200, Ingo Molnar wrote:
>
> * Alexey Dobriyan <[email protected]> wrote:
>
> > Use INT3 instead of NOP. All that padding between functions is
> > an illegal area, no legitimate code should jump into it.
> >
> > I've checked x86_64 allyesconfig disassembly, all changes looks sane:
> > INT3 is only used after RET or unconditional JMP.
> >
> > On i386:
> > * promote ret_from_exception into ENTRY as it has corresponding END,
> > * demote "resume_userspace" -- unused,
> > * delete ALIGN directive in page_fault. It is leftover from x86 assembly
> > cleanups.
> >
> > commit d211af055d0c12dc3416c2886e6fbdc6eb74a381
> > i386: get rid of the use of KPROBE_ENTRY / KPROBE_END
> >
> > has ALIGN directive before branch target which makes sense.
> > All the code after ALIGN disappeared later.
> >
> > Signed-off-by: Alexey Dobriyan <[email protected]>
> > ---
> >
> > arch/x86/entry/entry_32.S | 6 +-----
> > arch/x86/include/asm/linkage.h | 2 +-
> > 2 files changed, 2 insertions(+), 6 deletions(-)
> >
> > --- a/arch/x86/entry/entry_32.S
> > +++ b/arch/x86/entry/entry_32.S
> > @@ -320,8 +320,7 @@ END(ret_from_fork)
> > */
> >
> > # userspace resumption stub bypassing syscall exit tracing
> > - ALIGN
> > -ret_from_exception:
> > +ENTRY(ret_from_exception)
> > preempt_stop(CLBR_ANY)
> > ret_from_intr:
> > #ifdef CONFIG_VM86
> > @@ -337,8 +336,6 @@ ret_from_intr:
> > #endif
> > cmpl $USER_RPL, %eax
> > jb resume_kernel # not returning to v8086 or userspace
> > -
> > -ENTRY(resume_userspace)
> > DISABLE_INTERRUPTS(CLBR_ANY)
> > TRACE_IRQS_OFF
> > movl %esp, %eax
> > @@ -910,7 +907,6 @@ BUILD_INTERRUPT3(hv_stimer0_callback_vector, HYPERV_STIMER0_VECTOR,
> > ENTRY(page_fault)
> > ASM_CLAC
> > pushl $do_page_fault
> > - ALIGN
> > jmp common_exception
> > END(page_fault)
> >
> > --- a/arch/x86/include/asm/linkage.h
> > +++ b/arch/x86/include/asm/linkage.h
> > @@ -18,7 +18,7 @@
> > name:
> >
> > #if defined(CONFIG_X86_64) || defined(CONFIG_X86_ALIGNMENT_16)
> > -#define __ALIGN .p2align 4, 0x90
> > +#define __ALIGN .p2align 4, 0xCC
> > #define __ALIGN_STR __stringify(__ALIGN)
> > #endif
>
> So the question is, without objtool support, how will we find INT3-padding related
> crash bugs on 32-bit kernels?
Is the INT3 padding really worth it, even on x86-64? What problem are
we trying to solve?
I've seen cases with GCC functions falling through, but with asm code,
falling through could just be working as designed.
--
Josh
On Fri, May 18, 2018 at 12:27:15AM -0700, H. Peter Anvin wrote:
> On 05/18/18 00:18, Ingo Molnar wrote:
> >
> > Ok, this is cool, it addresses the robustness problem that INT3 padding introduced
> > very nicely.
> >
> > The concept of built-in kernel tooling working at the machine code level is just
> > so powerful - we should have added our own KCC compiler 20 years ago.
> >
>
> How many times has this been suggested? ;)
Goes to show we still really need it. Imagine the simplifications and
improvements we'll be able to do. Oh myyy....
/me has a wet dream.
--
Regards/Gruss,
Boris.
Good mailing practices for 400: avoid top-posting and trim the reply.
On Fri, May 18, 2018 at 08:02:24AM -0500, Josh Poimboeuf wrote:
> On Fri, May 18, 2018 at 09:36:44AM +0200, Ingo Molnar wrote:
> >
> > * Alexey Dobriyan <[email protected]> wrote:
> >
> > > Use INT3 instead of NOP. All that padding between functions is
> > > an illegal area, no legitimate code should jump into it.
> > >
> > > I've checked x86_64 allyesconfig disassembly, all changes looks sane:
> > > INT3 is only used after RET or unconditional JMP.
> > >
> > > On i386:
> > > * promote ret_from_exception into ENTRY as it has corresponding END,
> > > * demote "resume_userspace" -- unused,
> > > * delete ALIGN directive in page_fault. It is leftover from x86 assembly
> > > cleanups.
> > >
> > > commit d211af055d0c12dc3416c2886e6fbdc6eb74a381
> > > i386: get rid of the use of KPROBE_ENTRY / KPROBE_END
> > >
> > > has ALIGN directive before branch target which makes sense.
> > > All the code after ALIGN disappeared later.
> > >
> > > Signed-off-by: Alexey Dobriyan <[email protected]>
> > > ---
> > >
> > > arch/x86/entry/entry_32.S | 6 +-----
> > > arch/x86/include/asm/linkage.h | 2 +-
> > > 2 files changed, 2 insertions(+), 6 deletions(-)
> > >
> > > --- a/arch/x86/entry/entry_32.S
> > > +++ b/arch/x86/entry/entry_32.S
> > > @@ -320,8 +320,7 @@ END(ret_from_fork)
> > > */
> > >
> > > # userspace resumption stub bypassing syscall exit tracing
> > > - ALIGN
> > > -ret_from_exception:
> > > +ENTRY(ret_from_exception)
> > > preempt_stop(CLBR_ANY)
> > > ret_from_intr:
> > > #ifdef CONFIG_VM86
> > > @@ -337,8 +336,6 @@ ret_from_intr:
> > > #endif
> > > cmpl $USER_RPL, %eax
> > > jb resume_kernel # not returning to v8086 or userspace
> > > -
> > > -ENTRY(resume_userspace)
> > > DISABLE_INTERRUPTS(CLBR_ANY)
> > > TRACE_IRQS_OFF
> > > movl %esp, %eax
> > > @@ -910,7 +907,6 @@ BUILD_INTERRUPT3(hv_stimer0_callback_vector, HYPERV_STIMER0_VECTOR,
> > > ENTRY(page_fault)
> > > ASM_CLAC
> > > pushl $do_page_fault
> > > - ALIGN
> > > jmp common_exception
> > > END(page_fault)
> > >
> > > --- a/arch/x86/include/asm/linkage.h
> > > +++ b/arch/x86/include/asm/linkage.h
> > > @@ -18,7 +18,7 @@
> > > name:
> > >
> > > #if defined(CONFIG_X86_64) || defined(CONFIG_X86_ALIGNMENT_16)
> > > -#define __ALIGN .p2align 4, 0x90
> > > +#define __ALIGN .p2align 4, 0xCC
> > > #define __ALIGN_STR __stringify(__ALIGN)
> > > #endif
> >
> > So the question is, without objtool support, how will we find INT3-padding related
> > crash bugs on 32-bit kernels?
>
> Is the INT3 padding really worth it, even on x86-64? What problem are
> we trying to solve?
It is a start: manual padding, then compiler inserted padding, then
kernel CFI (in the future).
The only ways processor can end up in the padding are memory corruption,
exploit, some kind of miscompilation or CPU bug. In every case it is better
to crash immediately.
> I've seen cases with GCC functions falling through, but with asm code,
> falling through could just be working as designed.
Manual NOP align still can be inserted, fallthough is not the common case.
On Fri, May 18, 2018 at 09:18:14AM +0200, Ingo Molnar wrote:
> The concept of built-in kernel tooling working at the machine code level is just
> so powerful - we should have added our own KCC compiler 20 years ago.
...for two very serious reasons
* C as a language moves very slowly, last help from the comittee were
C99 intializers which are OK, but, say, memory model was explictly
rejected. However the project expands and becomes more complex much
faster than C working group sets up meetings. Compiler authors help
with extensions but ultimately can not be relied on (see "inline" saga).
Recently everyone was celebrating new and improved min() and max()
macros admiring creativity and knowledge of intricate language details
(me too, don't get this wrong).
Now this is how it can be done in a language which is not stupid:
constexpr int min(int a, int b)
{
return a < b ? a : b;
}
That's literally all. And you can also do
template<typename T>
void min(T a, char b) = delete;
template<typename T>
void min(char a, T b) = delete;
because "char" is char.
Having control over compiler things like that can be addded more
quickly.
* insulating the project from the whims of compiler authors who every
once in a while use "undefined behaviour" or other kinds of language
lawyering to do strange things.
Other serious projects do this too. Database people use O_DIRECT
to insulate themselves from kernel people for the very same reasons.
Hi!
> Side note: doing some grepping, I find some other sequences that are a bit
> scary, like this:
>
> arch/x86/kernel/acpi/wakeup_32.S-.data
> arch/x86/kernel/acpi/wakeup_32.S-ALIGN
> arch/x86/kernel/acpi/wakeup_32.S:ENTRY(saved_magic) .long 0
> arch/x86/kernel/acpi/wakeup_32.S:ENTRY(saved_eip) .long 0
>
> so apparently people are using ENTRY() for data too (the same pattern
> exists in wakeup_64.S).
>
> So we end up having those odd 0x90 bytes (now 0xcc) in the data section as
> "padding" between those two values. Crazy.
Sorry about that. I'm pretty sure intention was simply to use the
variable from C code.. and ENTRY() worked. I was not aware that it has
side effect of padding...
Let me see how this can be improved... (untested).
diff --git a/arch/x86/kernel/acpi/wakeup_32.S b/arch/x86/kernel/acpi/wakeup_32.S
index 0c26b1b..d6f477f 100644
--- a/arch/x86/kernel/acpi/wakeup_32.S
+++ b/arch/x86/kernel/acpi/wakeup_32.S
@@ -89,8 +89,8 @@ ret_point:
.data
ALIGN
-ENTRY(saved_magic) .long 0
-ENTRY(saved_eip) .long 0
+GLOBAL(saved_magic) .long 0
+saved_eip: .long 0
# saved registers
saved_idt: .long 0,0
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html
On May 18, 2018 10:51:36 AM PDT, Alexey Dobriyan <[email protected]> wrote:
>On Fri, May 18, 2018 at 09:18:14AM +0200, Ingo Molnar wrote:
>> The concept of built-in kernel tooling working at the machine code
>level is just
>> so powerful - we should have added our own KCC compiler 20 years ago.
>
>...for two very serious reasons
>
>* C as a language moves very slowly, last help from the comittee were
> C99 intializers which are OK, but, say, memory model was explictly
> rejected. However the project expands and becomes more complex much
> faster than C working group sets up meetings. Compiler authors help
>with extensions but ultimately can not be relied on (see "inline"
>saga).
>
> Recently everyone was celebrating new and improved min() and max()
> macros admiring creativity and knowledge of intricate language details
> (me too, don't get this wrong).
>
> Now this is how it can be done in a language which is not stupid:
>
> constexpr int min(int a, int b)
> {
> return a < b ? a : b;
> }
>
> That's literally all. And you can also do
>
> template<typename T>
> void min(T a, char b) = delete;
>
> template<typename T>
> void min(char a, T b) = delete;
>
> because "char" is char.
>
> Having control over compiler things like that can be addded more
> quickly.
>
>
>* insulating the project from the whims of compiler authors who every
> once in a while use "undefined behaviour" or other kinds of language
> lawyering to do strange things.
>
> Other serious projects do this too. Database people use O_DIRECT
> to insulate themselves from kernel people for the very same reasons.
Sounds like you are proposing switching to C++ more than anything else.
*Steps aside and grabs popcorn*
--
Sent from my Android device with K-9 Mail. Please excuse my brevity.
On Saturday, May 19, 2018 9:00:08 AM CEST Pavel Machek wrote:
> Hi!
>
> > Side note: doing some grepping, I find some other sequences that are a bit
> > scary, like this:
> >
> > arch/x86/kernel/acpi/wakeup_32.S-.data
> > arch/x86/kernel/acpi/wakeup_32.S-ALIGN
> > arch/x86/kernel/acpi/wakeup_32.S:ENTRY(saved_magic) .long 0
> > arch/x86/kernel/acpi/wakeup_32.S:ENTRY(saved_eip) .long 0
> >
> > so apparently people are using ENTRY() for data too (the same pattern
> > exists in wakeup_64.S).
> >
> > So we end up having those odd 0x90 bytes (now 0xcc) in the data section as
> > "padding" between those two values. Crazy.
>
> Sorry about that. I'm pretty sure intention was simply to use the
> variable from C code.. and ENTRY() worked. I was not aware that it has
> side effect of padding...
>
> Let me see how this can be improved... (untested).
>
> diff --git a/arch/x86/kernel/acpi/wakeup_32.S b/arch/x86/kernel/acpi/wakeup_32.S
> index 0c26b1b..d6f477f 100644
> --- a/arch/x86/kernel/acpi/wakeup_32.S
> +++ b/arch/x86/kernel/acpi/wakeup_32.S
> @@ -89,8 +89,8 @@ ret_point:
>
> .data
> ALIGN
> -ENTRY(saved_magic) .long 0
> -ENTRY(saved_eip) .long 0
> +GLOBAL(saved_magic) .long 0
> +saved_eip: .long 0
>
> # saved registers
> saved_idt: .long 0,0
The Jiri Slaby's annotation patches touch this:
https://patchwork.kernel.org/patch/10409073/
Thanks,
Rafael