From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from heliosphere.sirena.org.uk (heliosphere.sirena.org.uk [172.104.155.198]) by sourceware.org (Postfix) with ESMTPS id 73972388E811 for ; Thu, 10 Jun 2021 13:18:53 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 73972388E811 Received: from host86-167-10-84.range86-167.btcentralplus.com ([86.167.10.84] helo=fitzroy.sirena.org.uk) by heliosphere.sirena.org.uk with esmtpsa (TLS1.3) tls TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384 (Exim 4.94.2) (envelope-from ) id 1lrKa4-00AMCR-B9; Thu, 10 Jun 2021 13:18:44 +0000 Received: by fitzroy.sirena.org.uk (Postfix, from userid 1000) id 6F928D0E9DF; Thu, 10 Jun 2021 14:19:05 +0100 (BST) Date: Thu, 10 Jun 2021 14:19:05 +0100 From: Mark Brown To: Dave Martin Cc: Catalin Marinas , Will Deacon , Szabolcs Nagy , Jeremy Linton , "H . J . Lu" , Yu-cheng Yu , linux-arch@vger.kernel.org, linux-arm-kernel@lists.infradead.org, libc-alpha@sourceware.org Subject: Re: [PATCH v2 2/3] arm64: Enable BTI for main executable as well as the interpreter Message-ID: References: <20210604112450.13344-1-broonie@kernel.org> <20210604112450.13344-3-broonie@kernel.org> <20210609151713.GL4187@arm.com> MIME-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="E21DmkTzCZLD726x" Content-Disposition: inline In-Reply-To: <20210609151713.GL4187@arm.com> X-Cookie: A penny saved has not been spent. X-Spam-Status: No, score=1.5 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_EF, HEADER_FROM_DIFFERENT_DOMAINS, KAM_DMARC_NONE, RCVD_IN_BARRACUDACENTRAL, SPF_HELO_NONE, SPF_NONE autolearn=no autolearn_force=no version=3.4.2 X-Spam-Level: * X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: libc-alpha@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Libc-alpha mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 10 Jun 2021 13:18:58 -0000 --E21DmkTzCZLD726x Content-Type: text/plain; charset=us-ascii Content-Disposition: inline On Wed, Jun 09, 2021 at 04:17:13PM +0100, Dave Martin wrote: > On Fri, Jun 04, 2021 at 12:24:49PM +0100, Mark Brown wrote: > > - if (system_supports_bti() && has_interp == is_interp && > > - (*p & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)) > > - arch->flags |= ARM64_ELF_BTI; > > + if (system_supports_bti() && > > + (*p & GNU_PROPERTY_AARCH64_FEATURE_1_BTI)) { > > + if (is_interp) { > > + arch->flags |= ARM64_ELF_INTERP_BTI; > > + } else { > > + arch->flags |= ARM64_ELF_EXEC_BTI; > > + } > Nit: surplus curlies? (coding-style.rst does actually say to drop them > when all branches of an if are single-statement one-liners -- I had > presumed I was just being pedantic...) I really think this hurts readability with the nested if inside another if with a multi-line condition. > > - if (prot & PROT_EXEC) > > - prot |= PROT_BTI; > > + if (state->flags & ARM64_ELF_EXEC_BTI && !is_interp) > > + prot |= PROT_BTI; > > + } > Is it worth adding () around the bitwise-& expressions? I'm always a > little uneasy about the operator precedence of binary &, although > without looking it up I think you're correct. Sure. I'm fairly sure the compiler would've complained about this case if it were ambiguous, I'm vaguely surprised it didn't already. > Feel free to adopt if this appeals to you, otherwise I'm also fine with > your version.) I'll see what I think when I get back to looking at this properly. --E21DmkTzCZLD726x Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- iQEzBAABCgAdFiEEreZoqmdXGLWf4p/qJNaLcl1Uh9AFAmDCEUMACgkQJNaLcl1U h9B3oAf8DRt9uAEt6gqwDA1mGvwN/9TRY6qqVi2u9dKZwDznwg6UO/8RA4d6zpMa xMS1GemBG2J+9Ew1xy7c4TbNHIXCMfo1ZlHlnZmCmbUyDBm9oCTRZ0TBMxpNd3nC sG/ENNWllk/kZ4HGL0NI1+N9OlY8IROrSc8eaIre6ivN6aQWa6Q1IMtdJBqBOD0A P2MtLevPcxASaETGmZIr/mW2HMzCAlE1+NnfeVwS2fZRJWavEoJN3jfRnNgwn/N1 9Ot91bVhERHOHl9lNLSVtgQ/Odw1uph+4NArogEH1YE0IgHH9PmyvaKDRT4vyRR1 pU87HhEyr1AkrAQPeACcfHdN+M6WmA== =0xVB -----END PGP SIGNATURE----- --E21DmkTzCZLD726x--