linux-kernel - Re: [PATCH] powerpc/le: fix endianness of the arguments passed to the ppc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <1394084634.26321.23.camel@pasglop>
Date:	Thu, 06 Mar 2014 16:43:54 +1100
From:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
To:	Greg Kurz <gkurz@...ux.vnet.ibm.com>
Cc:	paulus@...ba.org, linuxppc-dev@...ts.ozlabs.org, anton@...ba.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] powerpc/le: fix endianness of the arguments passed to
 the ppc_rtas() syscall

On Wed, 2014-03-05 at 18:20 +0100, Greg Kurz wrote:
> RTAS manipulates its input and output arguments in big endian order.
> 
> I have looked at factoring some lines with rtas_call() but it is not really
> worth it because of the variable arguments.

I'm not sure about that one ...

So this assumes that userspace will pass the RTAS arguments LE...

This will work only as long as the arguments are indeed just an array
of u32's. The minute userspace tries to crap something like two u16's
in one argument or a u64 accross two arguments, it has the potential
to be broken depending on how it has been done.

I would have done it the other way around which is to define that
ppc_rtas takes a BE structure since this is what RTAS expects (and
the bit of RMA memory we expose to userspace for communicating with
it also has to be handled BE so userspace has to know anyway).

That means that the only things we need to fix here is when we interpret
the args.nargs etc... we need a be32_to_cpu

Cheers,
Ben.

> Signed-off-by: Greg Kurz <gkurz@...ux.vnet.ibm.com>
> ---
>  arch/powerpc/kernel/rtas.c |   15 +++++++++++++--
>  1 file changed, 13 insertions(+), 2 deletions(-)
> 
> diff --git a/arch/powerpc/kernel/rtas.c b/arch/powerpc/kernel/rtas.c
> index 4cf674d..00dbe36 100644
> --- a/arch/powerpc/kernel/rtas.c
> +++ b/arch/powerpc/kernel/rtas.c
> @@ -1020,6 +1020,7 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
>  	char *buff_copy, *errbuf = NULL;
>  	int nargs;
>  	int rc;
> +	int i;
>  
>  	if (!capable(CAP_SYS_ADMIN))
>  		return -EPERM;
> @@ -1056,9 +1057,19 @@ asmlinkage int ppc_rtas(struct rtas_args __user *uargs)
>  
>  	flags = lock_rtas();
>  
> -	rtas.args = args;
> +	rtas.args.token = cpu_to_be32(args.token);
> +	rtas.args.nargs = cpu_to_be32(args.nargs);
> +	rtas.args.nret  = cpu_to_be32(args.nret);
> +	for (i = 0; i < nargs; ++i)
> +		rtas.args.args[i] = cpu_to_be32(args.args[i]);
> +	rtas.args.rets  = &rtas.args.args[nargs];
> +	for (i = 0; i < args.nret; ++i)
> +		rtas.args.rets[i] = 0;
> +
>  	enter_rtas(__pa(&rtas.args));
> -	args = rtas.args;
> +
> +	for (i = 0; i < args.nret; ++i)
> +		args.rets[i] = be32_to_cpu(rtas.args.rets[i]);
>  
>  	/* A -1 return code indicates that the last command couldn't
>  	   be completed due to a hardware error. */


--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/