From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755997AbcLNOzO (ORCPT ); Wed, 14 Dec 2016 09:55:14 -0500 Received: from youngberry.canonical.com ([91.189.89.112]:60689 "EHLO youngberry.canonical.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755004AbcLNOzM (ORCPT ); Wed, 14 Dec 2016 09:55:12 -0500 Subject: Re: [PATCH] rcu: shift by 1UL rather than 1 to fix sign extension error To: Boqun Feng References: <20161213105646.9598-1-colin.king@canonical.com> <20161213112148.GE9728@tardis.cn.ibm.com> <3607072d-2418-df43-fd9a-2708a95f97da@canonical.com> <20161214144244.GL9728@tardis.cn.ibm.com> Cc: "Paul E . McKenney" , Josh Triplett , Steven Rostedt , Mathieu Desnoyers , Lai Jiangshan , linux-kernel@vger.kernel.org From: Colin Ian King Message-ID: <21d2e114-26a9-2eff-893b-9ed2295248ce@canonical.com> Date: Wed, 14 Dec 2016 14:54:34 +0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:45.0) Gecko/20100101 Thunderbird/45.3.0 MIME-Version: 1.0 In-Reply-To: <20161214144244.GL9728@tardis.cn.ibm.com> Content-Type: multipart/signed; micalg=pgp-sha256; protocol="application/pgp-signature"; boundary="UqvhrFlf9wUM6W0dIIQtJ35mU0xWKK48O" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org This is an OpenPGP/MIME signed message (RFC 4880 and 3156) --UqvhrFlf9wUM6W0dIIQtJ35mU0xWKK48O Content-Type: multipart/mixed; boundary="BWEGuIKraTR7LIXDSccroIqDPKLhE0hwi"; protected-headers="v1" From: Colin Ian King To: Boqun Feng Cc: "Paul E . McKenney" , Josh Triplett , Steven Rostedt , Mathieu Desnoyers , Lai Jiangshan , linux-kernel@vger.kernel.org Message-ID: <21d2e114-26a9-2eff-893b-9ed2295248ce@canonical.com> Subject: Re: [PATCH] rcu: shift by 1UL rather than 1 to fix sign extension error References: <20161213105646.9598-1-colin.king@canonical.com> <20161213112148.GE9728@tardis.cn.ibm.com> <3607072d-2418-df43-fd9a-2708a95f97da@canonical.com> <20161214144244.GL9728@tardis.cn.ibm.com> In-Reply-To: <20161214144244.GL9728@tardis.cn.ibm.com> --BWEGuIKraTR7LIXDSccroIqDPKLhE0hwi Content-Type: text/plain; charset=windows-1252 Content-Transfer-Encoding: quoted-printable On 14/12/16 14:42, Boqun Feng wrote: > 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. >>>> >>> >>> 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 >>> >>> 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 alon= g >>> with mine, does that work for you? >> >> 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 ;-) Yep, that's totally fine. Thanks. Colin >=20 > Regards, > Boqun >=20 >>> >>> TBH, this situation is kinda new to me, so if anyone has any suggesti= on, >>> 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= , >>>> =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 >=20 --BWEGuIKraTR7LIXDSccroIqDPKLhE0hwi-- --UqvhrFlf9wUM6W0dIIQtJ35mU0xWKK48O Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- iQI2BAEBCAAgBQJYUV0qGRxjb2xpbi5raW5nQGNhbm9uaWNhbC5jb20ACgkQaMKH 38aoAiby3g//euzNyPPcAEtdgqVADJpkq75fDerWE4tKEBJ9oCYNW0P0JYWdzE7N glt2/c1qUcaDE0vHFS+BDtpSktC1LR/sGhQPcgpxptb7ad25WrrQG0zVWBkJrEb+ S0O9rU7YN8veQJBhYZlJRVk4Gok8MtRAVj1xTaJWTWCcrrnHAKQegwrtOY16BmT4 u2/NjnoviHPv7/d34LH4QH7QeM7ebxSTdAnTCGZzgO5zSJ/Vn5iMh+ztisoR9UyP uukz344r+6sfjIOyX2SQRjbnX1ySr/GkGtbyODhzxuFf5SEMK4h6h9fKiiIoQIm0 Smj74WfKRQmTcPSXaAC81sUb2456wtCck90WPP4/06+6CXQS0WvO65yN0QaAo9Pc Vqm8foWe1CIX5kRAhpwV4e1VMcU8FexWnyqRQPGamtlQey50MFw/TAXSTe95pOeH 0VwEFCjarnpnAmWGYbQHXWUA/9GARZiB53hoRWSyRdLsuve69TQAtXRr+42Dt3t5 yK8aqw0rZzBD9z/wCeutok07bWKPq31yIimIyauzHdzAkoJfxM2IHHsrDTHcLk5r ynLLR0dDCh2a1E92RuWFfLADWahP9sAvYgJWAtwGJqonAJ0iFU2hTRZOX7B4eFCC QxvD22dX5nNKGbl95sfGR0jHNpjUWHkcd1siRtiz8yJTXoQbsPOv70U= =ji03 -----END PGP SIGNATURE----- --UqvhrFlf9wUM6W0dIIQtJ35mU0xWKK48O--