Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Fri, 30 Jun 2023 11:29:29 -0700
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
Message-ID: <202306301128.E8B5E51B5@keescook>
References: <20230629234244.1752366-8-samitolvanen@google.com>
 <20230629234244.1752366-9-samitolvanen@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230629234244.1752366-9-samitolvanen@google.com>
Precedence: bulk

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