Bug: Write fault blocked by KUAP! (kernel 6.2-rc6, Talos II)

[Christophe Leroy]

Le 03/02/2023 à 03:45, Nicholas Piggin a écrit :
> On Fri Feb 3, 2023 at 12:02 PM AEST, Benjamin Gray wrote:
>> On Fri, 2023-02-03 at 00:46 +0100, Erhard F. wrote:
>>> Happened during boot:
>>>
>>> [...]
>>> Creating 6 MTD partitions on "flash at 0":
>>> 0x000000000000-0x000004000000 : "PNOR"
>>> 0x000001b21000-0x000003921000 : "BOOTKERNEL"
>>> 0x000003a44000-0x000003a68000 : "CAPP"
>>> 0x000003a88000-0x000003a89000 : "VERSION"
>>> 0x000003a89000-0x000003ac9000 : "IMA_CATALOG"
>>> 0x000003e10000-0x000004000000 : "BOOTKERNFW"
>>> BTRFS info: devid 1 device path /dev/root changed to /dev/nvme0n1p3
>>> scanned by systemd-udevd (387)
>>> Kernel attempted to write user page (aa55c280000) - exploit attempt?
>>> (uid: 0)
>>> ------------[ cut here ]------------
>>> Bug: Write fault blocked by KUAP!
> 
> KUAP is a red herring of course, the KUAP test just checks if the
> faulting address is below TASK_SIZE.
> 
> [snip]
> 
>>> --- interrupt: 300 at __patch_instruction+0x50/0x70
>>> NIP:  c000000000064670 LR: c000000000064c2c CTR: c000000000048ee0
>>> REGS: c000000023b57630 TRAP: 0300   Tainted: G                T
>>> (6.2.0-rc6-P9)
>>> MSR:  900000000280b032 <SF,HV,VEC,VSX,EE,FP,ME,IR,DR,RI>  CR:
>>> 24222244  XER: 00000000
>>> CFAR: c00000000006462c DAR: 00000aa55c280000 DSISR: 42000000 IRQMASK:
>                                              ^^^^        ^^
> First byte of page, store, no PTE.
> 
>>> 1
>>> GPR00: 0000000000000000 c000000023b578d0 c000000000e7cc00
>>> c00800000ce33ffc
>>> GPR04: 041ae13000000000 00000aa55c27fffc 0000000000000000
>                                          ^^^^
> Last word of previous page.
> 
> Probably from create_stub function descriptor patching, which is not
> actually patching in an instruction so it probably gets unlucky and
> gets some data that matches prefix opcode and so it tries to store
> 8 bytes.

An easy fix would probably be to also check the suffix as a prefixed 
instruction with 0 as suffix is not valid :

diff --git a/arch/powerpc/include/asm/inst.h 
b/arch/powerpc/include/asm/inst.h
index 684d3f453282..87084a52598b 100644
--- a/arch/powerpc/include/asm/inst.h
+++ b/arch/powerpc/include/asm/inst.h
@@ -86,7 +86,7 @@ static inline ppc_inst_t ppc_inst_read(const u32 *ptr)

  static inline bool ppc_inst_prefixed(ppc_inst_t x)
  {
-	return IS_ENABLED(CONFIG_PPC64) && ppc_inst_primary_opcode(x) == 
OP_PREFIX;
+	return IS_ENABLED(CONFIG_PPC64) && ppc_inst_primary_opcode(x) == 
OP_PREFIX && ppc_inst_suffix(x);
  }

  static inline ppc_inst_t ppc_inst_swab(ppc_inst_t x)


> 
> So not your bug, your temp mm code just exposed it. Data shouldn't
> be patched using patch_instruction. We should have a patch_data_u32
> or similar that doesn't use instructions.

There is work in progress on this but if the above check works, it is 
probably a better approach as a fix that can be backported.