linux-kernel - Re: [PATCH] rcu: shift by 1UL rather than 1 to fix sign extension error

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: <20161213112148.GE9728@tardis.cn.ibm.com>
Date:   Tue, 13 Dec 2016 19:21:48 +0800
From:   Boqun Feng <boqun.feng@...il.com>
To:     Colin King <colin.king@...onical.com>
Cc:     "Paul E . McKenney" <paulmck@...ux.vnet.ibm.com>,
        Josh Triplett <josh@...htriplett.org>,
        Steven Rostedt <rostedt@...dmis.org>,
        Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
        Lai Jiangshan <jiangshanlai@...il.com>,
        linux-kernel@...r.kernel.org
Subject: Re: [PATCH] rcu: shift by 1UL rather than 1 to fix sign extension
 error

On Tue, Dec 13, 2016 at 10:56:46AM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@...onical.com>
> 
> mask and bit are unsigned longs, so if bit is 31 we end up sign
> extending the 1 and mask ends up as 0xffffffff80000000. Fix this
> by explicitly adding integer suffix UL ensure 1 is a unsigned long
> rather than an signed int.
> 

Right, you are, and the tool is ;-)

If @bit is greater than 32, we even got an undefined behavior in C ;-(
This is my careless mistake, thank you for finding it out and fix it!

> Issue found with static analysis with CoverityScan, CID 1388564
> 
> Fixes: 8965c3ce4718754db ("rcu: Use leaf_node_for_each_mask_possible_cpu() in force_qs_rnp()")
> Signed-off-by: Colin Ian King <colin.king@...onical.com>

I think Paul only queued that for running tests and I have almost
finished a v2. I will fold your fix in my patch and add your SoB along
with mine, does that work for you?

TBH, this situation is kinda new to me, so if anyone has any suggestion,
please let me know ;-)

Regards,
Boqun

> ---
>  kernel/rcu/tree.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 10162ac..6ecedd8 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -3051,7 +3051,7 @@ static void force_qs_rnp(struct rcu_state *rsp,
>  
>  		leaf_node_for_each_mask_possible_cpu(rnp, rnp->qsmask, bit, cpu)
>  			if (f(per_cpu_ptr(rsp->rda, cpu), isidle, maxj))
> -				mask |= 1 << bit;
> +				mask |= 1UL << bit;
>  
>  		if (mask != 0) {
>  			/* Idle/offline CPUs, report (releases rnp->lock. */
> -- 
> 2.10.2
> 

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