From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932771AbcLMM0C (ORCPT ); Tue, 13 Dec 2016 07:26:02 -0500 Received: from mail-pg0-f66.google.com ([74.125.83.66]:35526 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932455AbcLMM0B (ORCPT ); Tue, 13 Dec 2016 07:26:01 -0500 Date: Tue, 13 Dec 2016 20:25:42 +0800 From: Boqun Feng To: Colin King Cc: "Paul E . McKenney" , Josh Triplett , Steven Rostedt , Mathieu Desnoyers , Lai Jiangshan , linux-kernel@vger.kernel.org Subject: Re: [PATCH] rcu: shift by 1UL rather than 1 to fix sign extension error Message-ID: <20161213122542.GF9728@tardis.cn.ibm.com> References: <20161213105646.9598-1-colin.king@canonical.com> <20161213112148.GE9728@tardis.cn.ibm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="XStn23h1fwudRqtG" Content-Disposition: inline In-Reply-To: <20161213112148.GE9728@tardis.cn.ibm.com> User-Agent: Mutt/1.7.1 (2016-10-04) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --XStn23h1fwudRqtG Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Dec 13, 2016 at 07:21:48PM +0800, Boqun Feng wrote: > On Tue, Dec 13, 2016 at 10:56:46AM +0000, Colin King wrote: > > From: Colin Ian King > >=20 > > 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. > >=20 >=20 > Right, you are, and the tool is ;-) >=20 > 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! >=20 > > Issue found with static analysis with CoverityScan, CID 1388564 > >=20 > > Fixes: 8965c3ce4718754db ("rcu: Use leaf_node_for_each_mask_possible_cp= u() in force_qs_rnp()") > > Signed-off-by: Colin Ian King >=20 > 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? >=20 > TBH, this situation is kinda new to me, so if anyone has any suggestion, > please let me know ;-) >=20 > Regards, > Boqun >=20 > > --- > > kernel/rcu/tree.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > >=20 > > 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, > > =20 > > leaf_node_for_each_mask_possible_cpu(rnp, rnp->qsmask, bit, cpu) > > if (f(per_cpu_ptr(rsp->rda, cpu), isidle, maxj)) > > - mask |=3D 1 << bit; > > + mask |=3D 1UL << bit; Hmm.. Seems using BIT() here is a good idea, and maybe rename bit as grp_idx or something. Naming, naming, naming.. Regards, Boqun > > =20 > > if (mask !=3D 0) { > > /* Idle/offline CPUs, report (releases rnp->lock. */ > > --=20 > > 2.10.2 > >=20 --XStn23h1fwudRqtG Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEcBAABCAAGBQJYT+jDAAoJEEl56MO1B/q4KR8H/2zV61mrw4AQlEwX/XtNdCAC HbhVtlmMZ6lZ3oLX6wiL9HkVrtCPi8djRAwVrDQmiVpE+cKqEf7GtMaAjhc8wLMo vqcJotjkgEbH8KWEjb7ISevX81cA15thG+JQb0GywsoKiKggAmg5uN9/bgdYhWAY 27tbrcF443tpvzo3TNG65Fb2alWJ2yqZ1fHmGhgZNehEfkG7geqElotUQT8nWPLq S4q+h+TJPLOMe6VRLVSLJ+oRtHrftBuj+tGQ7nxhG6PkYfwBTGW8BUMQ92ZvM1WL xYdbtlSFwCNMrimslXtNnbOI6q3hd8rkaO50oyGUrP34frIP1EdBnuJVLu8wkNM= =wFNH -----END PGP SIGNATURE----- --XStn23h1fwudRqtG--