linux-kernel - Re: [PATCH 04/11] powerpc: Don't negate error in syscall_set_return

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <CAGXu5j+FA0KvJPr46QeJYzPpKYdNhzqnpa4kn5D_Mk2Hm=FTrA@mail.gmail.com>
Date:	Mon, 27 Jul 2015 11:49:51 -0700
From:	Kees Cook <keescook@...omium.org>
To:	Michael Ellerman <mpe@...erman.id.au>
Cc:	"linuxppc-dev@...abs.org" <linuxppc-dev@...abs.org>,
	LKML <linux-kernel@...r.kernel.org>,
	Andy Lutomirski <luto@...capital.net>,
	Will Drewry <wad@...omium.org>, strosake@...ux.vnet.ibm.com,
	bogdan.purcareata@...escale.com
Subject: Re: [PATCH 04/11] powerpc: Don't negate error in syscall_set_return_value()

On Thu, Jul 23, 2015 at 3:21 AM, Michael Ellerman <mpe@...erman.id.au> wrote:
> Currently the only caller of syscall_set_return_value() is seccomp
> filter, which is not enabled on powerpc.
>
> This means we have not noticed that our implementation of
> syscall_set_return_value() negates error, even though the value passed
> in is already negative.
>
> So remove the negation in syscall_set_return_value(), and expect the
> caller to do it like all other implementations do.
>
> Also add a comment about the ccr handling.
>
> Signed-off-by: Michael Ellerman <mpe@...erman.id.au>

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

-Kees

> ---
>  arch/powerpc/include/asm/syscall.h | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/arch/powerpc/include/asm/syscall.h b/arch/powerpc/include/asm/syscall.h
> index c6239dabcfb1..cabe90133e69 100644
> --- a/arch/powerpc/include/asm/syscall.h
> +++ b/arch/powerpc/include/asm/syscall.h
> @@ -44,9 +44,15 @@ static inline void syscall_set_return_value(struct task_struct *task,
>                                             struct pt_regs *regs,
>                                             int error, long val)
>  {
> +       /*
> +        * In the general case it's not obvious that we must deal with CCR
> +        * here, as the syscall exit path will also do that for us. However
> +        * there are some places, eg. the signal code, which check ccr to
> +        * decide if the value in r3 is actually an error.
> +        */
>         if (error) {
>                 regs->ccr |= 0x10000000L;
> -               regs->gpr[3] = -error;
> +               regs->gpr[3] = error;
>         } else {
>                 regs->ccr &= ~0x10000000L;
>                 regs->gpr[3] = val;
> --
> 2.1.0
>



-- 
Kees Cook
Chrome OS Security
--
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/