Messages in this thread | | | Date | Fri, 29 Oct 2021 12:15:01 -0700 | From | Pawan Gupta <> | Subject | Re: [PATCH v2 1/2] arch/Kconfig: Make CONFIG_CPU_SPECTRE available for all architectures |
| |
On 29.10.2021 18:05, Daniel Borkmann wrote: >On 10/29/21 11:22 AM, Mark Rutland wrote: >>On Thu, Oct 28, 2021 at 12:36:58PM -0700, Pawan Gupta wrote: >>>On 28.10.2021 14:49, Mark Rutland wrote: >>>>On Wed, Oct 27, 2021 at 06:33:22PM -0700, Pawan Gupta wrote: >>>>>Borrow CONFIG_CPU_SPECTRE from ARM to be available for all >>>>>architectures. This will help in configuration of features that depend >>>>>on CPU being affected by spectre class of vulnerabilities. >>>>> >>>>>Signed-off-by: Pawan Gupta <pawan.kumar.gupta@linux.intel.com> >>>> >>>>Given that spectre isn't one specific issue, biut rather a blanket term >>>>for a bunch of things that can have variable overlap, I don't think this >>>>makes much sense unless we're going to add finer-grained options for all >>>>the variants, and IMO it'd make more sene for the architectures to >>>>directly select the things that'd otherwise be dependent on this. >>> >>>Isn't ARM already using CPU_SPECTRE for selecting things: >>> >>> config HARDEN_BRANCH_PREDICTOR >>> bool "Harden the branch predictor against aliasing attacks" if EXPERT >>> depends on CPU_SPECTRE >> >>It's true that arch/arm does, but that's not true for other >>architectures, e.g. powerpc or arm64, and and as above I don't think it >>makes sense to make this generic in its current form because "spectre" >>is a somewhat vague generic term. >> >>>This was the whole motivation for doing the same for x86. >>> >>>Adding a condition for all architectures is also okay, but its going to >>>a little messier: >>> >>> config BPF_UNPRIV_DEFAULT_OFF >>> default y if X86 || ARM || ... >>> >>>This approach would make sense if architectures wants to explicitly >>>select the defaults irrespective of architecture being affected by >>>spectre. >> >>If we're going to change the default for some architectures, I think >>it'd make much more sense to just do that for all, without any >>arch-specific conditionality, i.e. >> >> config BPF_UNPRIV_DEFAULT_OFF >> default y > >Lets just go with 'default y'. The main rationale for this change was motivated >by spectre, so would have been good to indicate this also with an explicit >dependency for broken HW, not just help description. Pretty much agreeing with >Greg here [0]. Eventually, we might need some arch generic way to determine arch- >common spectre type bugs, so that for unaffected HW we don't need to apply some >of them from verifier, but that's still tbd.
I will send a patch soon with 'default y'.
Thanks, Pawan
| |