linux-kernel - Re: [PATCH v13 06/30] powerpc/ptrace: Adapt gpr32_get, gpr32

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: <877fbxqbvd.fsf@possimpible.ozlabs.ibm.com>
Date:	Thu, 04 Aug 2016 16:36:22 +1000
From:	Daniel Axtens <dja@...ens.net>
To:	wei.guo.simon@...il.com, linuxppc-dev@...ts.ozlabs.org,
	linux-kernel@...r.kernel.org
Cc:	Michael Ellerman <mpe@...erman.id.au>,
	Shuah Khan <shuahkh@....samsung.com>,
	Anton Blanchard <anton@...ba.org>,
	Cyril Bur <cyrilbur@...il.com>,
	Anshuman Khandual <khandual@...ux.vnet.ibm.com>,
	Simon Guo <wei.guo.simon@...il.com>,
	Ulrich Weigand <ulrich.weigand@...ibm.com>,
	Michael Neuling <mikey@...ling.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Kees Cook <keescook@...omium.org>,
	Rashmica Gupta <rashmicy@...il.com>,
	Khem Raj <raj.khem@...il.com>, Jessica Yu <jeyu@...hat.com>,
	Jiri Kosina <jkosina@...e.cz>, Miroslav Benes <mbenes@...e.cz>,
	Suraj Jitindar Singh <sjitindarsingh@...il.com>,
	Chris Smart <chris@...troguy.com>,
	linux-kselftest@...r.kernel.org
Subject: Re: [PATCH v13 06/30] powerpc/ptrace: Adapt gpr32_get, gpr32_set functions for transaction

Hi all,

This is causing cppcheck warnings (having just landed in next):

[arch/powerpc/kernel/ptrace.c:2062]: (error) Uninitialized variable: ckpt_regs
[arch/powerpc/kernel/ptrace.c:2130]: (error) Uninitialized variable: ckpt_regs

This is from...
> -static int gpr32_get(struct task_struct *target,
> +static int gpr32_get_common(struct task_struct *target,
>  		     const struct user_regset *regset,
>  		     unsigned int pos, unsigned int count,
> -		     void *kbuf, void __user *ubuf)
> +			    void *kbuf, void __user *ubuf, bool tm_active)
>  {
>  	const unsigned long *regs = &target->thread.regs->gpr[0];
> +	const unsigned long *ckpt_regs;
>  	compat_ulong_t *k = kbuf;
>  	compat_ulong_t __user *u = ubuf;
>  	compat_ulong_t reg;
>  	int i;
>  
> -	if (target->thread.regs == NULL)
> -		return -EIO;
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +	ckpt_regs = &target->thread.ckpt_regs.gpr[0];
> +#endif
> +	if (tm_active) {
> +		regs = ckpt_regs;
... this bit here. If the ifdef doesn't trigger, cppcheck can't find an
initialisation for ckpt_regs, so it complains.

Techinically it's a false positive as (I assume!) tm_active cannot ever
be true in the absense of CONFIG_PPC_TRANSACTIONAL_MEM.

Is there a nice simple fix we could deploy to squash this warning, or
will we just live with it?

> -static int gpr32_set(struct task_struct *target,
> +static int gpr32_set_common(struct task_struct *target,
>  		     const struct user_regset *regset,
>  		     unsigned int pos, unsigned int count,
> -		     const void *kbuf, const void __user *ubuf)
> +		     const void *kbuf, const void __user *ubuf, bool tm_active)
>  {
>  	unsigned long *regs = &target->thread.regs->gpr[0];
> +	unsigned long *ckpt_regs;
>  	const compat_ulong_t *k = kbuf;
>  	const compat_ulong_t __user *u = ubuf;
>  	compat_ulong_t reg;
>  
> -	if (target->thread.regs == NULL)
> -		return -EIO;
> +#ifdef CONFIG_PPC_TRANSACTIONAL_MEM
> +	ckpt_regs = &target->thread.ckpt_regs.gpr[0];
> +#endif
>  
> -	CHECK_FULL_REGS(target->thread.regs);
> +	if (tm_active) {
> +		regs = ckpt_regs;
FWIW it happens again here.

Regards,
Daniel Axtens

Download attachment "signature.asc" of type "application/pgp-signature" (860 bytes)