On 08/03/2022, 14:50:47, Nicholas Piggin wrote:
> Add range checking from the rtas syscall, and other error checks
> and warnings to kernel callers, so problems can be found and
> fixed.
>
> Signed-off-by: Nicholas Piggin <npig...@gmail.com>
> ---
> arch/powerpc/kernel/rtas.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index adf4892aeecd..7f8a3fd685f9 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -428,6 +428,23 @@ static int notrace va_raw_rtas_call(struct rtas_args
> *args, int token,
> {
> int i;
>
> + if (!irqs_disabled()) {
> + WARN_ON_ONCE(1);
> + return -1;
> + }
> +
> + if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE) {
> + WARN_ON_ONCE(1);
> + return -1;
> + }
> +
> + if (nargs >= ARRAY_SIZE(args->args)
> + || nret > ARRAY_SIZE(args->args)
> + || nargs + nret > ARRAY_SIZE(args->args)) {
> + WARN_ON_ONCE(1);
> + return -1;
> + }
These 3 tests are making the function returning -1, which is previously
only returned in the case the call cannot be achieved because of a hardware
error (as stated in rtas_call()).
Should a dedicated error code been returned here?
> +
> args->token = cpu_to_be32(token);
> args->nargs = cpu_to_be32(nargs);
> args->nret = cpu_to_be32(nret);
> @@ -476,9 +493,6 @@ int rtas_call(int token, int nargs, int nret, int
> *outputs, ...)
> char *buff_copy = NULL;
> int ret;
>
> - if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
> - return -1;
> -
> if ((mfmsr() & (MSR_IR|MSR_DR)) != (MSR_IR|MSR_DR)) {
> WARN_ON_ONCE(1);
> return -1;
> @@ -955,9 +969,6 @@ int rtas_call_reentrant(int token, int nargs, int nret,
> int *outputs, ...)
> unsigned long flags;
> int ret;
>
> - if (!rtas.entry || token == RTAS_UNKNOWN_SERVICE)
> - return -1;
> -
> local_irq_save(flags);
> preempt_disable();
>
