All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dan Li <ashimida.1990@gmail.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Masahiro Yamada <masahiroy@kernel.org>,
	Michal Marek <michal.lkml@markovi.net>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Sami Tolvanen <samitolvanen@google.com>,
	Kees Cook <keescook@chromium.org>,
	Nathan Chancellor <nathan@kernel.org>, Tom Rix <trix@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Josh Poimboeuf <jpoimboe@kernel.org>,
	Frederic Weisbecker <frederic@kernel.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Marco Elver <elver@google.com>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Song Liu <song@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Uros Bizjak <ubizjak@gmail.com>,
	Kumar Kartikeya Dwivedi <memxor@gmail.com>,
	Juergen Gross <jgross@suse.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Borislav Petkov <bp@suse.de>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Aaron Tomlin <atomlin@redhat.com>,
	Kalesh Singh <kaleshsingh@google.com>,
	Yuntao Wang <ytcoode@gmail.com>,
	Changbin Du <changbin.du@intel.com>,
	linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, llvm@lists.linux.dev
Subject: Re: [RFC/RFT] CFI: Add support for gcc CFI in aarch64
Date: Sat, 7 Jan 2023 07:37:19 -0800	[thread overview]
Message-ID: <20230107153719.cpuq5yrc7v67f2uy@ubuntu> (raw)
In-Reply-To: <Y7Ps+R4Oqx3xiaVH@FVFF77S0Q05N>

Hi Mark,

Sorry for the late reply.

On 01/03, Mark Rutland wrote:
> On Sun, Dec 18, 2022 at 10:17:58PM -0800, Dan Li wrote:
> >    If there are user mode programs or other systems that want to use
> >    this feature, it may be more convenient to use a callback (so this
> >    compilation option is set to -fsanitize=cfi instead of kcfi).
> 
> I appreciate that may be nicer for userspace, but it would be far nicer for the
> kernel if we could have a kcfi mode that behaves the same as LLVM, using a BRK.
> That's going to be simpler for the kernel to deal with, and should result in
> nicer code / smaller binary size (for the reasons given above).
> 
> Can we please have an LLVM-compatible KCFI mode, and have the -fsanitize=cfi be
> a separate option from -fsanitize=kcfi?

Ok, in the next version I will change to the same option as clang :)

> 
> > 2. A reserved typeid (such as 0x0U on the aarch64 platform) is always
> >    inserted in front of functions that should not be called indirectly.
> >    Functions that can be called indirectly will not use this hash value,
> >    which prevents instructions/data before the function from being used
> >    as a typeid by an attacker.
> 
> That sounds sensible, though it meanse we'll need to go audit all the assembly
> without type annotations.
> 
> I presume that "functions that should not be called indirectly" only includes
> those which are not directly visible outside the compilation unit AND whose
> address is never taken / escaped from the compilation unit. Is that the case?

Yes.

> 
> > 3. Some bits are ignored in the typeid to avoid conflicts between the
> >    typeid and the instruction set of a specific platform, thereby
> >    preventing an attacker from bypassing the CFI check by using the
> >    instruction as a typeid, such as on the aarch64 platform:
> >    * If the following instruction sequence exists:
> > 	  400620:       a9be7bfd        stp     x29, x30, [sp, #-32]!
> > 	  400624:       910003fd        mov     x29, sp
> > 	  400628:       f9000bf3        str     x19, [sp, #16]
> >    * If the expected typeid of the indirect call is exactly 0x910003fd,
> >      the attacker can jump to the next instruction position of any
> >      "mov x29,sp" instruction (such as 0x400628 here).
> 
> Which bits exactly are ignored on arm64?
> 
> e.g. are these encoded into UDF immediates?

In aarch64, I currently ignore bit [28:27]. IUCC, according to the manual[1],
it is a UDF instruction only when the upper 16 bits are all 0.
But due to this has too much impact on the entropy of typeid, so I (not
rigorously) only ignore 2 bits here, and most of the instruction codes covered
by it belong to 'Reserved' or 'UNALLOCATED' (probably not a good idea).

But as Kees said, if clang doesn't handle it here, in order to be consistent,
I think it's better for gcc to not handle it when implementing kernel cfi.

[1] https://developer.arm.com/documentation/ddi0602/2022-06/Index-by-Encoding?lang=en

> 
> As a general thing, how does this work with -fpatchable-function-entry=M,N,
> where N is non-zero?
> 
> We still need to fix that for LLVM, and it would be good to align on the same behaviour.
>

Yeah, it makes sense.

Currently, it is consistent with llvm. Taking -fpatchable-function-entry=2,1
as an example, the currently generated code is as follows:

__cfi_main:
        .4byte 0x439d3502
        .global main
        .section        __patchable_function_entries
        .align  3
        .8byte  .LPFE3
        .text
.LPFE3:
        nop
        .type   main, %function
main:
        nop
.LFB2:
        .cfi_startproc
        stp     x29, x30, [sp, -32]!

Finally, do we want to generate code like this?
        nop
        .4byte 0x439d3502
main:
        nop
        ...

Thanks,
Dan.

> >  
> > -- 
> > 2.17.1
> > 

WARNING: multiple messages have this Message-ID (diff)
From: Dan Li <ashimida.1990@gmail.com>
To: Mark Rutland <mark.rutland@arm.com>
Cc: Masahiro Yamada <masahiroy@kernel.org>,
	Michal Marek <michal.lkml@markovi.net>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Will Deacon <will@kernel.org>,
	Sami Tolvanen <samitolvanen@google.com>,
	Kees Cook <keescook@chromium.org>,
	Nathan Chancellor <nathan@kernel.org>, Tom Rix <trix@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	"Paul E. McKenney" <paulmck@kernel.org>,
	Josh Poimboeuf <jpoimboe@kernel.org>,
	Frederic Weisbecker <frederic@kernel.org>,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Marco Elver <elver@google.com>,
	Christophe Leroy <christophe.leroy@csgroup.eu>,
	Song Liu <song@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Uros Bizjak <ubizjak@gmail.com>,
	Kumar Kartikeya Dwivedi <memxor@gmail.com>,
	Juergen Gross <jgross@suse.com>,
	Luis Chamberlain <mcgrof@kernel.org>,
	Borislav Petkov <bp@suse.de>,
	Masami Hiramatsu <mhiramat@kernel.org>,
	Dmitry Torokhov <dmitry.torokhov@gmail.com>,
	Aaron Tomlin <atomlin@redhat.com>,
	Kalesh Singh <kaleshsingh@google.com>,
	Yuntao Wang <ytcoode@gmail.com>,
	Changbin Du <changbin.du@intel.com>,
	linux-kbuild@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-arm-kernel@lists.infradead.org, llvm@lists.linux.dev
Subject: Re: [RFC/RFT] CFI: Add support for gcc CFI in aarch64
Date: Sat, 7 Jan 2023 07:37:19 -0800	[thread overview]
Message-ID: <20230107153719.cpuq5yrc7v67f2uy@ubuntu> (raw)
In-Reply-To: <Y7Ps+R4Oqx3xiaVH@FVFF77S0Q05N>

Hi Mark,

Sorry for the late reply.

On 01/03, Mark Rutland wrote:
> On Sun, Dec 18, 2022 at 10:17:58PM -0800, Dan Li wrote:
> >    If there are user mode programs or other systems that want to use
> >    this feature, it may be more convenient to use a callback (so this
> >    compilation option is set to -fsanitize=cfi instead of kcfi).
> 
> I appreciate that may be nicer for userspace, but it would be far nicer for the
> kernel if we could have a kcfi mode that behaves the same as LLVM, using a BRK.
> That's going to be simpler for the kernel to deal with, and should result in
> nicer code / smaller binary size (for the reasons given above).
> 
> Can we please have an LLVM-compatible KCFI mode, and have the -fsanitize=cfi be
> a separate option from -fsanitize=kcfi?

Ok, in the next version I will change to the same option as clang :)

> 
> > 2. A reserved typeid (such as 0x0U on the aarch64 platform) is always
> >    inserted in front of functions that should not be called indirectly.
> >    Functions that can be called indirectly will not use this hash value,
> >    which prevents instructions/data before the function from being used
> >    as a typeid by an attacker.
> 
> That sounds sensible, though it meanse we'll need to go audit all the assembly
> without type annotations.
> 
> I presume that "functions that should not be called indirectly" only includes
> those which are not directly visible outside the compilation unit AND whose
> address is never taken / escaped from the compilation unit. Is that the case?

Yes.

> 
> > 3. Some bits are ignored in the typeid to avoid conflicts between the
> >    typeid and the instruction set of a specific platform, thereby
> >    preventing an attacker from bypassing the CFI check by using the
> >    instruction as a typeid, such as on the aarch64 platform:
> >    * If the following instruction sequence exists:
> > 	  400620:       a9be7bfd        stp     x29, x30, [sp, #-32]!
> > 	  400624:       910003fd        mov     x29, sp
> > 	  400628:       f9000bf3        str     x19, [sp, #16]
> >    * If the expected typeid of the indirect call is exactly 0x910003fd,
> >      the attacker can jump to the next instruction position of any
> >      "mov x29,sp" instruction (such as 0x400628 here).
> 
> Which bits exactly are ignored on arm64?
> 
> e.g. are these encoded into UDF immediates?

In aarch64, I currently ignore bit [28:27]. IUCC, according to the manual[1],
it is a UDF instruction only when the upper 16 bits are all 0.
But due to this has too much impact on the entropy of typeid, so I (not
rigorously) only ignore 2 bits here, and most of the instruction codes covered
by it belong to 'Reserved' or 'UNALLOCATED' (probably not a good idea).

But as Kees said, if clang doesn't handle it here, in order to be consistent,
I think it's better for gcc to not handle it when implementing kernel cfi.

[1] https://developer.arm.com/documentation/ddi0602/2022-06/Index-by-Encoding?lang=en

> 
> As a general thing, how does this work with -fpatchable-function-entry=M,N,
> where N is non-zero?
> 
> We still need to fix that for LLVM, and it would be good to align on the same behaviour.
>

Yeah, it makes sense.

Currently, it is consistent with llvm. Taking -fpatchable-function-entry=2,1
as an example, the currently generated code is as follows:

__cfi_main:
        .4byte 0x439d3502
        .global main
        .section        __patchable_function_entries
        .align  3
        .8byte  .LPFE3
        .text
.LPFE3:
        nop
        .type   main, %function
main:
        nop
.LFB2:
        .cfi_startproc
        stp     x29, x30, [sp, -32]!

Finally, do we want to generate code like this?
        nop
        .4byte 0x439d3502
main:
        nop
        ...

Thanks,
Dan.

> >  
> > -- 
> > 2.17.1
> > 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

  reply	other threads:[~2023-01-07 15:37 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2022-12-19  6:17 [RFC/RFT] CFI: Add support for gcc CFI in aarch64 Dan Li
2022-12-19  6:17 ` Dan Li
2022-12-19  6:38 ` Dan Li
2022-12-19  6:38   ` Dan Li
2022-12-19 10:40 ` Peter Zijlstra
2022-12-19 10:40   ` Peter Zijlstra
2022-12-19 13:32   ` Dan Li
2022-12-19 13:32     ` Dan Li
2022-12-19 15:04     ` Peter Zijlstra
2022-12-19 15:04       ` Peter Zijlstra
2022-12-19 23:38       ` Dan Li
2022-12-19 23:38         ` Dan Li
2023-01-03  8:55       ` Mark Rutland
2023-01-03  8:55         ` Mark Rutland
2023-01-07  3:36     ` Kees Cook
2023-01-07  3:36       ` Kees Cook
2023-01-07 15:42       ` Dan Li
2023-01-07 15:42         ` Dan Li
2023-02-08 19:35         ` Kees Cook
2023-02-08 19:35           ` Kees Cook
2023-02-10 16:14           ` Dan Li
2023-02-10 16:14             ` Dan Li
2023-01-03  8:53 ` Mark Rutland
2023-01-03  8:53   ` Mark Rutland
2023-01-07 15:37   ` Dan Li [this message]
2023-01-07 15:37     ` Dan Li
2023-03-25  8:54 ` [RFC/RFT,V2] " Dan Li
2023-03-27  9:30   ` Peter Zijlstra
2023-03-27 22:17     ` Sami Tolvanen
2023-04-05 11:49       ` Dan Li
2023-12-13  8:48       ` Dan Li
     [not found]         ` <4a84af95-6270-6764-6a40-875ec20fc3e1@lixiang.com>
2023-12-13 14:45           ` Mark Rutland
2023-12-13 19:35           ` Kees Cook
2023-04-05 11:48     ` Dan Li
2023-12-13  8:28 ` [RFC/RFT] " Dan Li
2023-12-13  8:28   ` Dan Li

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=20230107153719.cpuq5yrc7v67f2uy@ubuntu \
    --to=ashimida.1990@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=atomlin@redhat.com \
    --cc=bp@suse.de \
    --cc=catalin.marinas@arm.com \
    --cc=changbin.du@intel.com \
    --cc=christophe.leroy@csgroup.eu \
    --cc=dmitry.torokhov@gmail.com \
    --cc=ebiederm@xmission.com \
    --cc=elver@google.com \
    --cc=frederic@kernel.org \
    --cc=jgross@suse.com \
    --cc=jpoimboe@kernel.org \
    --cc=kaleshsingh@google.com \
    --cc=keescook@chromium.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=mark.rutland@arm.com \
    --cc=masahiroy@kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=memxor@gmail.com \
    --cc=mhiramat@kernel.org \
    --cc=michal.lkml@markovi.net \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=paulmck@kernel.org \
    --cc=peterz@infradead.org \
    --cc=samitolvanen@google.com \
    --cc=song@kernel.org \
    --cc=trix@redhat.com \
    --cc=ubizjak@gmail.com \
    --cc=will@kernel.org \
    --cc=ytcoode@gmail.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.