From: Kees Cook <keescook@chromium.org>
To: Sami Tolvanen <samitolvanen@google.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
Nathan Chancellor <nathan@kernel.org>,
Nick Desaulniers <ndesaulniers@google.com>,
linux-riscv@lists.infradead.org, llvm@lists.linux.dev,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/6] riscv: Add CFI error handling
Date: Fri, 30 Jun 2023 11:26:59 -0700 [thread overview]
Message-ID: <202306301126.D7EC815A22@keescook> (raw)
In-Reply-To: <20230629234244.1752366-12-samitolvanen@google.com>
On Thu, Jun 29, 2023 at 11:42:49PM +0000, Sami Tolvanen wrote:
> With CONFIG_CFI_CLANG, the compiler injects a type preamble immediately
> before each function and a check to validate the target function type
> before indirect calls:
>
> ; type preamble
> .word <id>
> function:
> ...
> ; indirect call check
> lw t1, -4(a0)
> lui t2, <hi20>
> addiw t2, t2, <lo12>
> beq t1, t2, .Ltmp0
> ebreak
> .Ltmp0:
> jarl a0
>
> Implement error handling code for the ebreak traps emitted for the
> checks. This produces the following oops on a CFI failure (generated
> using lkdtm):
>
> [ 21.177245] CFI failure at lkdtm_indirect_call+0x22/0x32 [lkdtm]
> (target: lkdtm_increment_int+0x0/0x18 [lkdtm]; expected type: 0x3ad55aca)
> [ 21.178483] Kernel BUG [#1]
> [ 21.178671] Modules linked in: lkdtm
> [ 21.179037] CPU: 1 PID: 104 Comm: sh Not tainted
> 6.3.0-rc6-00037-g37d5ec6297ab #1
> [ 21.179511] Hardware name: riscv-virtio,qemu (DT)
> [ 21.179818] epc : lkdtm_indirect_call+0x22/0x32 [lkdtm]
> [ 21.180106] ra : lkdtm_CFI_FORWARD_PROTO+0x48/0x7c [lkdtm]
> [ 21.180426] epc : ffffffff01387092 ra : ffffffff01386f14 sp : ff20000000453cf0
> [ 21.180792] gp : ffffffff81308c38 tp : ff6000000243f080 t0 : ff20000000453b78
> [ 21.181157] t1 : 000000003ad55aca t2 : 000000007e0c52a5 s0 : ff20000000453d00
> [ 21.181506] s1 : 0000000000000001 a0 : ffffffff0138d170 a1 : ffffffff013870bc
> [ 21.181819] a2 : b5fea48dd89aa700 a3 : 0000000000000001 a4 : 0000000000000fff
> [ 21.182169] a5 : 0000000000000004 a6 : 00000000000000b7 a7 : 0000000000000000
> [ 21.182591] s2 : ff20000000453e78 s3 : ffffffffffffffea s4 : 0000000000000012
> [ 21.183001] s5 : ff600000023c7000 s6 : 0000000000000006 s7 : ffffffff013882a0
> [ 21.183653] s8 : 0000000000000008 s9 : 0000000000000002 s10: ffffffff0138d878
> [ 21.184245] s11: ffffffff0138d878 t3 : 0000000000000003 t4 : 0000000000000000
> [ 21.184591] t5 : ffffffff8133df08 t6 : ffffffff8133df07
> [ 21.184858] status: 0000000000000120 badaddr: 0000000000000000
> cause: 0000000000000003
> [ 21.185415] [<ffffffff01387092>] lkdtm_indirect_call+0x22/0x32 [lkdtm]
> [ 21.185772] [<ffffffff01386f14>] lkdtm_CFI_FORWARD_PROTO+0x48/0x7c [lkdtm]
> [ 21.186093] [<ffffffff01383552>] lkdtm_do_action+0x22/0x34 [lkdtm]
> [ 21.186445] [<ffffffff0138350c>] direct_entry+0x128/0x13a [lkdtm]
> [ 21.186817] [<ffffffff8033ed8c>] full_proxy_write+0x58/0xb2
> [ 21.187352] [<ffffffff801d4fe8>] vfs_write+0x14c/0x33a
> [ 21.187644] [<ffffffff801d5328>] ksys_write+0x64/0xd4
> [ 21.187832] [<ffffffff801d53a6>] sys_write+0xe/0x1a
> [ 21.188171] [<ffffffff80003996>] ret_from_syscall+0x0/0x2
> [ 21.188595] Code: 0513 0f65 a303 ffc5 53b7 7e0c 839b 2a53 0363 0073 (9002) 9582
> [ 21.189178] ---[ end trace 0000000000000000 ]---
> [ 21.189590] Kernel panic - not syncing: Fatal exception
>
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Looks good -- should the noaddr failure paths include any warnings of
their own? (i.e. isn't that unexpected?)
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
_______________________________________________
linux-riscv mailing list
linux-riscv@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-riscv
WARNING: multiple messages have this Message-ID (diff)
From: Kees Cook <keescook@chromium.org>
To: Sami Tolvanen <samitolvanen@google.com>
Cc: Paul Walmsley <paul.walmsley@sifive.com>,
Palmer Dabbelt <palmer@dabbelt.com>,
Albert Ou <aou@eecs.berkeley.edu>,
Nathan Chancellor <nathan@kernel.org>,
Nick Desaulniers <ndesaulniers@google.com>,
linux-riscv@lists.infradead.org, llvm@lists.linux.dev,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH 4/6] riscv: Add CFI error handling
Date: Fri, 30 Jun 2023 11:26:59 -0700 [thread overview]
Message-ID: <202306301126.D7EC815A22@keescook> (raw)
In-Reply-To: <20230629234244.1752366-12-samitolvanen@google.com>
On Thu, Jun 29, 2023 at 11:42:49PM +0000, Sami Tolvanen wrote:
> With CONFIG_CFI_CLANG, the compiler injects a type preamble immediately
> before each function and a check to validate the target function type
> before indirect calls:
>
> ; type preamble
> .word <id>
> function:
> ...
> ; indirect call check
> lw t1, -4(a0)
> lui t2, <hi20>
> addiw t2, t2, <lo12>
> beq t1, t2, .Ltmp0
> ebreak
> .Ltmp0:
> jarl a0
>
> Implement error handling code for the ebreak traps emitted for the
> checks. This produces the following oops on a CFI failure (generated
> using lkdtm):
>
> [ 21.177245] CFI failure at lkdtm_indirect_call+0x22/0x32 [lkdtm]
> (target: lkdtm_increment_int+0x0/0x18 [lkdtm]; expected type: 0x3ad55aca)
> [ 21.178483] Kernel BUG [#1]
> [ 21.178671] Modules linked in: lkdtm
> [ 21.179037] CPU: 1 PID: 104 Comm: sh Not tainted
> 6.3.0-rc6-00037-g37d5ec6297ab #1
> [ 21.179511] Hardware name: riscv-virtio,qemu (DT)
> [ 21.179818] epc : lkdtm_indirect_call+0x22/0x32 [lkdtm]
> [ 21.180106] ra : lkdtm_CFI_FORWARD_PROTO+0x48/0x7c [lkdtm]
> [ 21.180426] epc : ffffffff01387092 ra : ffffffff01386f14 sp : ff20000000453cf0
> [ 21.180792] gp : ffffffff81308c38 tp : ff6000000243f080 t0 : ff20000000453b78
> [ 21.181157] t1 : 000000003ad55aca t2 : 000000007e0c52a5 s0 : ff20000000453d00
> [ 21.181506] s1 : 0000000000000001 a0 : ffffffff0138d170 a1 : ffffffff013870bc
> [ 21.181819] a2 : b5fea48dd89aa700 a3 : 0000000000000001 a4 : 0000000000000fff
> [ 21.182169] a5 : 0000000000000004 a6 : 00000000000000b7 a7 : 0000000000000000
> [ 21.182591] s2 : ff20000000453e78 s3 : ffffffffffffffea s4 : 0000000000000012
> [ 21.183001] s5 : ff600000023c7000 s6 : 0000000000000006 s7 : ffffffff013882a0
> [ 21.183653] s8 : 0000000000000008 s9 : 0000000000000002 s10: ffffffff0138d878
> [ 21.184245] s11: ffffffff0138d878 t3 : 0000000000000003 t4 : 0000000000000000
> [ 21.184591] t5 : ffffffff8133df08 t6 : ffffffff8133df07
> [ 21.184858] status: 0000000000000120 badaddr: 0000000000000000
> cause: 0000000000000003
> [ 21.185415] [<ffffffff01387092>] lkdtm_indirect_call+0x22/0x32 [lkdtm]
> [ 21.185772] [<ffffffff01386f14>] lkdtm_CFI_FORWARD_PROTO+0x48/0x7c [lkdtm]
> [ 21.186093] [<ffffffff01383552>] lkdtm_do_action+0x22/0x34 [lkdtm]
> [ 21.186445] [<ffffffff0138350c>] direct_entry+0x128/0x13a [lkdtm]
> [ 21.186817] [<ffffffff8033ed8c>] full_proxy_write+0x58/0xb2
> [ 21.187352] [<ffffffff801d4fe8>] vfs_write+0x14c/0x33a
> [ 21.187644] [<ffffffff801d5328>] ksys_write+0x64/0xd4
> [ 21.187832] [<ffffffff801d53a6>] sys_write+0xe/0x1a
> [ 21.188171] [<ffffffff80003996>] ret_from_syscall+0x0/0x2
> [ 21.188595] Code: 0513 0f65 a303 ffc5 53b7 7e0c 839b 2a53 0363 0073 (9002) 9582
> [ 21.189178] ---[ end trace 0000000000000000 ]---
> [ 21.189590] Kernel panic - not syncing: Fatal exception
>
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>
Looks good -- should the noaddr failure paths include any warnings of
their own? (i.e. isn't that unexpected?)
Reviewed-by: Kees Cook <keescook@chromium.org>
--
Kees Cook
next prev parent reply other threads:[~2023-06-30 18:27 UTC|newest]
Thread overview: 40+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-29 23:42 [PATCH 0/6] riscv: KCFI support Sami Tolvanen
2023-06-29 23:42 ` Sami Tolvanen
2023-06-29 23:42 ` [PATCH 1/6] riscv: Implement syscall wrappers Sami Tolvanen
2023-06-29 23:42 ` Sami Tolvanen
2023-06-30 18:29 ` Kees Cook
2023-06-30 18:29 ` Kees Cook
2023-06-29 23:42 ` [PATCH 2/6] riscv: Add types to indirectly called assembly functions Sami Tolvanen
2023-06-29 23:42 ` Sami Tolvanen
2023-06-30 18:25 ` Kees Cook
2023-06-30 18:25 ` Kees Cook
2023-06-29 23:42 ` [PATCH 3/6] riscv: Add ftrace_stub_graph Sami Tolvanen
2023-06-29 23:42 ` Sami Tolvanen
2023-06-30 18:25 ` Kees Cook
2023-06-30 18:25 ` Kees Cook
2023-06-29 23:42 ` [PATCH 4/6] riscv: Add CFI error handling Sami Tolvanen
2023-06-29 23:42 ` Sami Tolvanen
2023-06-30 18:26 ` Kees Cook [this message]
2023-06-30 18:26 ` Kees Cook
2023-06-30 19:03 ` Conor Dooley
2023-06-30 19:03 ` Conor Dooley
2023-06-29 23:42 ` [PATCH 5/6] riscv/purgatory: Disable CFI Sami Tolvanen
2023-06-29 23:42 ` Sami Tolvanen
2023-06-30 18:27 ` Kees Cook
2023-06-30 18:27 ` Kees Cook
2023-06-29 23:42 ` [PATCH 6/6] riscv: Allow CONFIG_CFI_CLANG to be selected Sami Tolvanen
2023-06-29 23:42 ` Sami Tolvanen
2023-06-30 18:27 ` Kees Cook
2023-06-30 18:27 ` Kees Cook
2023-06-30 19:07 ` Conor Dooley
2023-06-30 19:07 ` Conor Dooley
2023-07-05 20:43 ` Sami Tolvanen
2023-07-05 20:43 ` Sami Tolvanen
2023-06-30 18:48 ` [PATCH 0/6] riscv: KCFI support Conor Dooley
2023-06-30 18:48 ` Conor Dooley
2023-06-30 19:45 ` Conor Dooley
2023-06-30 19:45 ` Conor Dooley
2023-07-05 20:41 ` Sami Tolvanen
2023-07-05 20:41 ` Sami Tolvanen
2023-06-30 20:13 ` Nathan Chancellor
2023-06-30 20:13 ` Nathan Chancellor
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=202306301126.D7EC815A22@keescook \
--to=keescook@chromium.org \
--cc=aou@eecs.berkeley.edu \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-riscv@lists.infradead.org \
--cc=llvm@lists.linux.dev \
--cc=nathan@kernel.org \
--cc=ndesaulniers@google.com \
--cc=palmer@dabbelt.com \
--cc=paul.walmsley@sifive.com \
--cc=samitolvanen@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.