From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S932392AbcLNOnE (ORCPT ); Wed, 14 Dec 2016 09:43:04 -0500 Received: from mail-pg0-f66.google.com ([74.125.83.66]:33020 "EHLO mail-pg0-f66.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755004AbcLNOnD (ORCPT ); Wed, 14 Dec 2016 09:43:03 -0500 Date: Wed, 14 Dec 2016 22:42:44 +0800 From: Boqun Feng To: Colin Ian 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: <20161214144244.GL9728@tardis.cn.ibm.com> References: <20161213105646.9598-1-colin.king@canonical.com> <20161213112148.GE9728@tardis.cn.ibm.com> <3607072d-2418-df43-fd9a-2708a95f97da@canonical.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="ai3I8gwHc37+ASRI" Content-Disposition: inline In-Reply-To: <3607072d-2418-df43-fd9a-2708a95f97da@canonical.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 --ai3I8gwHc37+ASRI Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Dec 13, 2016 at 11:33:19AM +0000, Colin Ian King wrote: > On 13/12/16 11:21, Boqun Feng wrote: > > On Tue, Dec 13, 2016 at 10:56:46AM +0000, Colin King wrote: > >> From: Colin Ian King > >> > >> 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 > > 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 > >> > >> Fixes: 8965c3ce4718754db ("rcu: Use leaf_node_for_each_mask_possible_c= pu() 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 > Sure, that's good with me. >=20 Colin, as I'm going to take Mark's suggestion and use leaf_node_cpu_bit() instead. So I'm going to drop your SoB but keep a commit message saying you spotted the problem at the first place. Hope that works with you ;-) Regards, Boqun > >=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(-) > >> > >> 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; > >> =20 > >> if (mask !=3D 0) { > >> /* Idle/offline CPUs, report (releases rnp->lock. */ > >> --=20 > >> 2.10.2 > >> >=20 >=20 --ai3I8gwHc37+ASRI Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEcBAABCAAGBQJYUVpgAAoJEEl56MO1B/q4AoQH/AsXYmZjD51Y4rdrtsQO8k5o xRhPmr8NeefbsDPnrnaWW9tsAokQYmSmqjWfJZdKSf+X25w6kR/k0aI6PNartAz3 p9WKBPuy7DqR8vnqB3Ts/TlEf1rsSQt9HXrRobKdRfyk54AUSz+x38l1maGqHX/B uIDOg1dvp9prumJiq17KZEd0LQ81ujHUYQ/DpwwgO20+MjercrAOa6hCJIyyfYI8 TUfRcf1y+bBCtTN1BBaiER15w3EkgnamVtXzVIQZLLfQiyb85khPm2VYU6tzo4+4 Gmf8s6A7t8m8Uc7XZj4QauZWxuovXrb8J7HHelfB5qie4fu4812faGHPdurjr5I= =M/aF -----END PGP SIGNATURE----- --ai3I8gwHc37+ASRI--