Re: [PATCH v2 2/3] arm64: Enable BTI for main executable as well as the interpreter
From: Mark Brown <broonie@kernel.org>
Date: 2021-06-10 13:21:45
Also in:
linux-arch
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:
quoted
- 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.
quoted
- 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.