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

[Boqun Feng <boqun.feng@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1667 bytes --]

On Tue, Dec 13, 2016 at 10:56:46AM +0000, Colin King wrote:
> From: Colin Ian King <colin.king@canonical.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@canonical.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
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 455 bytes --]