From: Ingo Molnar <mingo@kernel.org> To: Dan Williams <dan.j.williams@intel.com> Cc: Thomas Gleixner <tglx@linutronix.de>, linux-arch <linux-arch@vger.kernel.org>, Cyril Novikov <cnovikov@lynx.com>, Kernel Hardening <kernel-hardening@lists.openwall.com>, Peter Zijlstra <peterz@infradead.org>, Catalin Marinas <catalin.marinas@arm.com>, X86 ML <x86@kernel.org>, Will Deacon <will.deacon@arm.com>, Russell King <linux@armlinux.org.uk>, Ingo Molnar <mingo@redhat.com>, Greg KH <gregkh@linuxfoundation.org>, "H. Peter Anvin" <hpa@zytor.com>, Linus Torvalds <torvalds@linux-foundation.org>, Alan Cox <alan@linux.intel.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org> Subject: Re: [PATCH v5 02/12] array_idx: sanitize speculative array de-references Date: Sun, 28 Jan 2018 19:33:09 +0100 [thread overview] Message-ID: <20180128183309.j7zkoyqblich4zhq@gmail.com> (raw) In-Reply-To: <CAPcyv4jTaskPT1qXhdCO2mWVkcaMDMBq12vUJ_hSfjOU49qvgQ@mail.gmail.com> * Dan Williams <dan.j.williams@intel.com> wrote: > Thomas, Peter, and Alexei wanted s/nospec_barrier/ifence/ and I just checked past discussions, and I cannot find that part, got any links or message-IDs? PeterZ's feedback on Jan 8 was: > On Sun, Jan 07, 2018 at 06:24:11PM -0800, Alexei Starovoitov wrote: > > How about: > > CONFIG_SPECTRE1_WORKAROUND_INDEX_MASK > > CONFIG_SPECTRE1_WORKAROUND_LOAD_FENCE > > INSTRUCTION_FENCE if anything. LFENCE for Intel (and now also for AMD as > per 0592b0bce169) is a misnomer, IFENCE would be a better name for it. Which in that context clearly talked about the config space and how to name the instruction semantics in light of the confusion of LFENCE and MFENCE opcodes on Intel and AMD CPUs... Also, those early reviews were fundamentally low level feedback related to the actual functionality of the barriers and their mapping to the hardware. But the fact is, the current series introduces an inconsistent barrier namespace extension of: barrier() barrier_data() mb() rmb() wmb() store_mb() read_barrier_depends() ... + ifence() + array_idx() + array_idx_mask() This isn't bikeshed painting: _ALL_ existing barrier API names have 'barrier' or its abbreviation 'mb' (memory barrier) somewhere in their names, which makes it pretty easy to recognize them at a glance. I'm giving you high level API naming feedback because we are now growing the size of the barrier API. array_idx() on the other hand is pretty much close to a 'worst possible' name: - it's named in an overly generic, opaque fashion - doesn't indicate it at all that it's a barrier for something ... and since we want all kernel developers to use these facilities correctly, we want the names to be good and suggestive as well. I'd accept pretty much anything else that adds 'barrier' or 'nospec' to the name: array_idx_barrier() or array_idx_nospec(). (I'm slightly leaning towards 'nospec' because it's more indicative of what is being done, and it also is what we do for get uaccess APIs.) ifence() is a similar departure from existing barrier naming nomenclature, and I'd accept pretty much any other variant: barrier_nospec() ifence_nospec() The kernel namespace cleanliness rules are clear and consistent, and there's nothing new about them: - the name of the API should unambiguously refer back to the API category. For barriers this common reference is 'barrier' or 'mb'. - use postfixes or prefixes consistently: pick one and don't mix them. If we go with a _nospec() variant for the uaccess API names then we should use a similar naming for array_idx() and for the new barrier as well - no mixing. > You can always follow on with a patch to fix up the names and placements to your > liking. While they'll pick on my name choices, they won't pick on yours, because > I simply can't be bothered to care about a bikeshed color at this point after > being bounced around for 5 revisions of this patch set. Sorry, this kind of dismissive and condescending attitude won't cut it. Thanks, Ingo
WARNING: multiple messages have this Message-ID (diff)
From: Ingo Molnar <mingo@kernel.org> To: Dan Williams <dan.j.williams@intel.com> Cc: Thomas Gleixner <tglx@linutronix.de>, linux-arch <linux-arch@vger.kernel.org>, Cyril Novikov <cnovikov@lynx.com>, Kernel Hardening <kernel-hardening@lists.openwall.com>, Peter Zijlstra <peterz@infradead.org>, Catalin Marinas <catalin.marinas@arm.com>, X86 ML <x86@kernel.org>, Will Deacon <will.deacon@arm.com>, Russell King <linux@armlinux.org.uk>, Ingo Molnar <mingo@redhat.com>, Greg KH <gregkh@linuxfoundation.org>, "H. Peter Anvin" <hpa@zytor.com>, Linus Torvalds <torvalds@linux-foundation.org>, Alan Cox <alan@linux.intel.com>, Linux Kernel Mailing List <linux-kernel@vger.kernel.org> Subject: [kernel-hardening] Re: [PATCH v5 02/12] array_idx: sanitize speculative array de-references Date: Sun, 28 Jan 2018 19:33:09 +0100 [thread overview] Message-ID: <20180128183309.j7zkoyqblich4zhq@gmail.com> (raw) In-Reply-To: <CAPcyv4jTaskPT1qXhdCO2mWVkcaMDMBq12vUJ_hSfjOU49qvgQ@mail.gmail.com> * Dan Williams <dan.j.williams@intel.com> wrote: > Thomas, Peter, and Alexei wanted s/nospec_barrier/ifence/ and I just checked past discussions, and I cannot find that part, got any links or message-IDs? PeterZ's feedback on Jan 8 was: > On Sun, Jan 07, 2018 at 06:24:11PM -0800, Alexei Starovoitov wrote: > > How about: > > CONFIG_SPECTRE1_WORKAROUND_INDEX_MASK > > CONFIG_SPECTRE1_WORKAROUND_LOAD_FENCE > > INSTRUCTION_FENCE if anything. LFENCE for Intel (and now also for AMD as > per 0592b0bce169) is a misnomer, IFENCE would be a better name for it. Which in that context clearly talked about the config space and how to name the instruction semantics in light of the confusion of LFENCE and MFENCE opcodes on Intel and AMD CPUs... Also, those early reviews were fundamentally low level feedback related to the actual functionality of the barriers and their mapping to the hardware. But the fact is, the current series introduces an inconsistent barrier namespace extension of: barrier() barrier_data() mb() rmb() wmb() store_mb() read_barrier_depends() ... + ifence() + array_idx() + array_idx_mask() This isn't bikeshed painting: _ALL_ existing barrier API names have 'barrier' or its abbreviation 'mb' (memory barrier) somewhere in their names, which makes it pretty easy to recognize them at a glance. I'm giving you high level API naming feedback because we are now growing the size of the barrier API. array_idx() on the other hand is pretty much close to a 'worst possible' name: - it's named in an overly generic, opaque fashion - doesn't indicate it at all that it's a barrier for something ... and since we want all kernel developers to use these facilities correctly, we want the names to be good and suggestive as well. I'd accept pretty much anything else that adds 'barrier' or 'nospec' to the name: array_idx_barrier() or array_idx_nospec(). (I'm slightly leaning towards 'nospec' because it's more indicative of what is being done, and it also is what we do for get uaccess APIs.) ifence() is a similar departure from existing barrier naming nomenclature, and I'd accept pretty much any other variant: barrier_nospec() ifence_nospec() The kernel namespace cleanliness rules are clear and consistent, and there's nothing new about them: - the name of the API should unambiguously refer back to the API category. For barriers this common reference is 'barrier' or 'mb'. - use postfixes or prefixes consistently: pick one and don't mix them. If we go with a _nospec() variant for the uaccess API names then we should use a similar naming for array_idx() and for the new barrier as well - no mixing. > You can always follow on with a patch to fix up the names and placements to your > liking. While they'll pick on my name choices, they won't pick on yours, because > I simply can't be bothered to care about a bikeshed color at this point after > being bounced around for 5 revisions of this patch set. Sorry, this kind of dismissive and condescending attitude won't cut it. Thanks, Ingo
next prev parent reply other threads:[~2018-01-28 18:33 UTC|newest] Thread overview: 75+ messages / expand[flat|nested] mbox.gz Atom feed top 2018-01-27 7:55 [PATCH v5 00/12] spectre variant1 mitigations for tip/x86/pti Dan Williams 2018-01-27 7:55 ` [kernel-hardening] " Dan Williams 2018-01-27 7:55 ` Dan Williams 2018-01-27 7:55 ` [PATCH v5 01/12] Documentation: document array_idx Dan Williams 2018-01-27 7:55 ` [kernel-hardening] " Dan Williams 2018-01-27 7:55 ` [PATCH v5 02/12] array_idx: sanitize speculative array de-references Dan Williams 2018-01-27 7:55 ` [kernel-hardening] " Dan Williams 2018-01-28 8:55 ` Ingo Molnar 2018-01-28 8:55 ` [kernel-hardening] " Ingo Molnar 2018-01-28 11:36 ` Thomas Gleixner 2018-01-28 11:36 ` [kernel-hardening] " Thomas Gleixner 2018-01-28 16:28 ` Dan Williams 2018-01-28 16:28 ` [kernel-hardening] " Dan Williams 2018-01-28 18:33 ` Ingo Molnar [this message] 2018-01-28 18:33 ` Ingo Molnar 2018-01-29 16:45 ` Dan Williams 2018-01-28 18:36 ` [kernel-hardening] " Thomas Gleixner 2018-01-28 18:36 ` Thomas Gleixner 2018-01-30 6:29 ` Dan Williams 2018-01-30 19:38 ` Linus Torvalds 2018-01-30 20:13 ` Dan Williams 2018-01-30 20:27 ` Van De Ven, Arjan 2018-01-31 8:03 ` Ingo Molnar 2018-01-31 14:13 ` Van De Ven, Arjan 2018-01-31 14:21 ` Greg KH 2018-01-27 7:55 ` [PATCH v5 03/12] x86: implement array_idx_mask Dan Williams 2018-01-27 7:55 ` [kernel-hardening] " Dan Williams 2018-01-28 9:02 ` Ingo Molnar 2018-01-28 9:02 ` [kernel-hardening] " Ingo Molnar 2018-01-27 7:55 ` [PATCH v5 04/12] x86: introduce __uaccess_begin_nospec and ifence Dan Williams 2018-01-27 7:55 ` [kernel-hardening] " Dan Williams 2018-01-28 9:06 ` Ingo Molnar 2018-01-28 9:06 ` [kernel-hardening] " Ingo Molnar 2018-01-28 9:14 ` Ingo Molnar 2018-01-28 9:14 ` [kernel-hardening] " Ingo Molnar 2018-01-29 20:41 ` Dan Williams 2018-01-30 6:56 ` Ingo Molnar 2018-01-27 7:55 ` [PATCH v5 05/12] x86, __get_user: use __uaccess_begin_nospec Dan Williams 2018-01-27 7:55 ` [kernel-hardening] " Dan Williams 2018-01-28 9:19 ` Ingo Molnar 2018-01-28 9:19 ` [kernel-hardening] " Ingo Molnar 2018-01-27 7:55 ` [PATCH v5 06/12] x86, get_user: use pointer masking to limit speculation Dan Williams 2018-01-27 7:55 ` [kernel-hardening] " Dan Williams 2018-01-28 9:25 ` Ingo Molnar 2018-01-28 9:25 ` [kernel-hardening] " Ingo Molnar 2018-01-27 7:55 ` [PATCH v5 07/12] x86: remove the syscall_64 fast-path Dan Williams 2018-01-27 7:55 ` [kernel-hardening] " Dan Williams 2018-01-28 9:29 ` Ingo Molnar 2018-01-28 9:29 ` [kernel-hardening] " Ingo Molnar 2018-01-28 15:22 ` Andy Lutomirski 2018-01-28 15:22 ` [kernel-hardening] " Andy Lutomirski 2018-01-27 7:55 ` [PATCH v5 08/12] x86: sanitize sycall table de-references under speculation Dan Williams 2018-01-27 7:55 ` [kernel-hardening] " Dan Williams 2018-01-28 9:36 ` Ingo Molnar 2018-01-28 9:36 ` [kernel-hardening] " Ingo Molnar 2018-01-27 7:56 ` [PATCH v5 09/12] vfs, fdtable: prevent bounds-check bypass via speculative execution Dan Williams 2018-01-27 7:56 ` [kernel-hardening] " Dan Williams 2018-01-27 7:56 ` [PATCH v5 10/12] kvm, x86: update spectre-v1 mitigation Dan Williams 2018-01-27 7:56 ` [kernel-hardening] " Dan Williams 2018-01-27 7:56 ` [PATCH v5 11/12] nl80211: sanitize array index in parse_txq_params Dan Williams 2018-01-27 7:56 ` [kernel-hardening] " Dan Williams 2018-01-27 7:56 ` Dan Williams 2018-01-27 7:56 ` [PATCH v5 12/12] x86/spectre: report get_user mitigation for spectre_v1 Dan Williams 2018-01-27 7:56 ` [kernel-hardening] " Dan Williams 2018-01-28 9:50 ` Ingo Molnar 2018-01-28 9:50 ` [kernel-hardening] " Ingo Molnar 2018-01-29 22:05 ` Dan Williams 2018-01-31 8:07 ` Ingo Molnar 2018-02-01 20:23 ` Dan Williams 2018-01-27 18:52 ` [PATCH v5 00/12] spectre variant1 mitigations for tip/x86/pti Linus Torvalds 2018-01-27 18:52 ` [kernel-hardening] " Linus Torvalds 2018-01-27 18:52 ` Linus Torvalds 2018-01-27 19:26 ` Dan Williams 2018-01-27 19:26 ` [kernel-hardening] " Dan Williams 2018-01-27 19:26 ` Dan Williams
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20180128183309.j7zkoyqblich4zhq@gmail.com \ --to=mingo@kernel.org \ --cc=alan@linux.intel.com \ --cc=catalin.marinas@arm.com \ --cc=cnovikov@lynx.com \ --cc=dan.j.williams@intel.com \ --cc=gregkh@linuxfoundation.org \ --cc=hpa@zytor.com \ --cc=kernel-hardening@lists.openwall.com \ --cc=linux-arch@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux@armlinux.org.uk \ --cc=mingo@redhat.com \ --cc=peterz@infradead.org \ --cc=tglx@linutronix.de \ --cc=torvalds@linux-foundation.org \ --cc=will.deacon@arm.com \ --cc=x86@kernel.org \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.