linux-kernel - Re: [PATCH 5/5] futex: Silence uninitialized warnings

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]

Date:	Mon, 13 Jan 2014 09:28:48 -0800
From:	Davidlohr Bueso <davidlohr@...com>
To:	Ingo Molnar <mingo@...nel.org>
Cc:	linux-kernel@...r.kernel.org, dvhart@...ux.intel.com,
	peterz@...radead.org, tglx@...utronix.de,
	paulmck@...ux.vnet.ibm.com, efault@....de, jeffm@...e.com,
	torvalds@...ux-foundation.org, jason.low2@...com,
	Waiman.Long@...com, tom.vaden@...com, scott.norton@...com,
	aswin@...com
Subject: Re: [PATCH 5/5] futex: Silence uninitialized warnings

On Mon, 2014-01-13 at 11:16 +0100, Ingo Molnar wrote:
> * Davidlohr Bueso <davidlohr@...com> wrote:
> 
> > From: Davidlohr Bueso <davidlohr@...com>
> > 
> > Callers of cmpxchg_futex_value_locked() can trigger the following:
> > 
> > kernel/futex.c: In function ‘futex_lock_pi_atomic’:
> > kernel/futex.c:725: warning: ‘curval’ may be used uninitialized in this function
> > 
> > This was initially addressed by commit 7cfdaf38, but others still remain. Silence
> > these messages once and for all as the variables really aren't uninitialized.
> > 
> > Cc: Ingo Molnar <mingo@...nel.org>
> > Acked-by: Darren Hart <dvhart@...ux.intel.com>
> > Cc: Peter Zijlstra <peterz@...radead.org>
> > Cc: Thomas Gleixner <tglx@...utronix.de>
> > Cc: Paul E. McKenney <paulmck@...ux.vnet.ibm.com>
> > Cc: Mike Galbraith <efault@....de>
> > Cc: Jeff Mahoney <jeffm@...e.com>
> > Cc: Linus Torvalds <torvalds@...ux-foundation.org>
> > Cc: Scott Norton <scott.norton@...com>
> > Cc: Tom Vaden <tom.vaden@...com>
> > Cc: Aswin Chandramouleeswaran <aswin@...com>
> > Cc: Waiman Long <Waiman.Long@...com>
> > Cc: Jason Low <jason.low2@...com>
> > Signed-off-by: Davidlohr Bueso <davidlohr@...com>
> > ---
> >  kernel/futex.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/kernel/futex.c b/kernel/futex.c
> > index be6399a..8d40953 100644
> > --- a/kernel/futex.c
> > +++ b/kernel/futex.c
> > @@ -838,7 +838,7 @@ static int futex_lock_pi_atomic(u32 __user *uaddr, struct futex_hash_bucket *hb,
> >  				struct task_struct *task, int set_waiters)
> >  {
> >  	int lock_taken, ret, force_take = 0;
> > -	u32 uval, newval, curval, vpid = task_pid_vnr(task);
> > +	u32 uval, newval, uninitialized_var(curval), vpid = task_pid_vnr(task);
> >  
> >  retry:
> >  	ret = lock_taken = 0;
> > @@ -2227,7 +2227,7 @@ static int futex_unlock_pi(u32 __user *uaddr, unsigned int flags)
> >  	struct futex_hash_bucket *hb;
> >  	struct futex_q *this, *next;
> >  	union futex_key key = FUTEX_KEY_INIT;
> > -	u32 uval, vpid = task_pid_vnr(current);
> > +	u32 uninitialized_var(uval), vpid = task_pid_vnr(current);
> >  	int ret;
> >  
> >  retry:
> > @@ -2843,7 +2843,7 @@ SYSCALL_DEFINE6(futex, u32 __user *, uaddr, int, op, u32, val,
> >  
> >  static int __init futex_init(void)
> >  {
> > -	u32 curval;
> > +	u32 uninitialized_var(curval);
> >  	unsigned long i;
> >  
> >  #if CONFIG_BASE_SMALL
> 
> I'm skipping this patch though.
> 
> I consider the use of uninitialized_var() dangerous and broken: if for 
> whatever reason a future change to the code makes the warning trigger 
> and makes it _true_, then we won't notice it because it's hidden 
> unconditionally ...

Sure, so we should then get rid of the already existing ones from
7cfdaf38.

> 
> The following alternative measures can be used to make spurious 
> old-compiler warnings go away:
> 
>  - Consider upgrading your compiler.
> 
>  - If for whatever reason you can't upgrade your compiler then
>    restructure the code so that the flow of logic is more apparent 
>    even to older GCC versions. (Chances are that the flow will be more 
>    readable to humans too, so it's a win-win!)
> 
>  - If you think the flow is exactly perfect and (older) GCC which you
>    cannot upgrade is still being silly, then initialize the variable
>    to zero. On a new compiler this won't mean a thing because GCC will 
>    notice the superfluous initialization and will optimize it out -
>    and it's a lot safer than just shutting the warning up forever.

Ok, we could go wit this last path then.

--
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/