All of lore.kernel.org
 help / color / mirror / Atom feed
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 1/6] riscv: Implement syscall wrappers
Date: Fri, 30 Jun 2023 11:29:29 -0700	[thread overview]
Message-ID: <202306301128.E8B5E51B5@keescook> (raw)
In-Reply-To: <20230629234244.1752366-9-samitolvanen@google.com>

On Thu, Jun 29, 2023 at 11:42:46PM +0000, Sami Tolvanen wrote:
> Commit f0bddf50586d ("riscv: entry: Convert to generic entry") moved
> syscall handling to C code, which exposed function pointer type
> mismatches that trip fine-grained forward-edge Control-Flow Integrity
> (CFI) checks as syscall handlers are all called through the same
> syscall_t pointer type. To fix the type mismatches, implement pt_regs
> based syscall wrappers similarly to x86 and arm64.
> 
> This patch is based on arm64 syscall wrappers added in commit
> 4378a7d4be30 ("arm64: implement syscall wrappers"), where the main goal
> was to minimize the risk of userspace-controlled values being used
> under speculation. This may be a concern for riscv in future as well.
> 
> Following other architectures, the syscall wrappers generate three
> functions for each syscall; __riscv_<compat_>sys_<name> takes a pt_regs
> pointer and extracts arguments from registers, __se_<compat_>sys_<name>
> is a sign-extension wrapper that casts the long arguments to the
> correct types for the real syscall implementation, which is named
> __do_<compat_>sys_<name>.
> 
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>

This all looks correct to me; though I have not run tested it. I'm glad
to see another arch using this style.

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 1/6] riscv: Implement syscall wrappers
Date: Fri, 30 Jun 2023 11:29:29 -0700	[thread overview]
Message-ID: <202306301128.E8B5E51B5@keescook> (raw)
In-Reply-To: <20230629234244.1752366-9-samitolvanen@google.com>

On Thu, Jun 29, 2023 at 11:42:46PM +0000, Sami Tolvanen wrote:
> Commit f0bddf50586d ("riscv: entry: Convert to generic entry") moved
> syscall handling to C code, which exposed function pointer type
> mismatches that trip fine-grained forward-edge Control-Flow Integrity
> (CFI) checks as syscall handlers are all called through the same
> syscall_t pointer type. To fix the type mismatches, implement pt_regs
> based syscall wrappers similarly to x86 and arm64.
> 
> This patch is based on arm64 syscall wrappers added in commit
> 4378a7d4be30 ("arm64: implement syscall wrappers"), where the main goal
> was to minimize the risk of userspace-controlled values being used
> under speculation. This may be a concern for riscv in future as well.
> 
> Following other architectures, the syscall wrappers generate three
> functions for each syscall; __riscv_<compat_>sys_<name> takes a pt_regs
> pointer and extracts arguments from registers, __se_<compat_>sys_<name>
> is a sign-extension wrapper that casts the long arguments to the
> correct types for the real syscall implementation, which is named
> __do_<compat_>sys_<name>.
> 
> Signed-off-by: Sami Tolvanen <samitolvanen@google.com>

This all looks correct to me; though I have not run tested it. I'm glad
to see another arch using this style.

Reviewed-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

  reply	other threads:[~2023-06-30 18:29 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 [this message]
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
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=202306301128.E8B5E51B5@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.