Re: [PATCH] x86/speculation: Fix user-mode spectre-v2 protection with KERNEL_IBRS

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Feb 20, 2023 at 3:38 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
>
> On Mon, Feb 20, 2023 at 03:25:08AM -0800, KP Singh wrote:
> > On Mon, Feb 20, 2023 at 3:20 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > On Mon, Feb 20, 2023 at 03:11:24AM -0800, KP Singh wrote:
> > > > On Mon, Feb 20, 2023 at 2:52 AM Greg KH <gregkh@xxxxxxxxxxxxxxxxxxx> wrote:
> > > > >
> > > > > On Mon, Feb 20, 2023 at 11:39:30AM +0100, KP Singh wrote:
> > > > > > With the introduction of KERNEL_IBRS, STIBP is no longer needed
> > > > > > to prevent cross thread training in the kernel space. When KERNEL_IBRS
> > > > > > was added, it also disabled the user-mode protections for spectre_v2.
> > > > > > KERNEL_IBRS does not mitigate cross thread training in the userspace.
> > > > > >
> > > > > > In order to demonstrate the issue, one needs to avoid syscalls in the
> > > > > > victim as syscalls can shorten the window size due to
> > > > > > a user -> kernel -> user transition which sets the
> > > > > > IBRS bit when entering kernel space and clearing any training the
> > > > > > attacker may have done.
> > > > > >
> > > > > > Allow users to select a spectre_v2_user mitigation (STIBP always on,
> > > > > > opt-in via prctl) when KERNEL_IBRS is enabled.
> > > > > >
> > > > > > Reported-by: José Oliveira <joseloliveira11@xxxxxxxxx>
> > > > > > Reported-by: Rodrigo Branco <rodrigo@xxxxxxxxxxxxxxxxx>
> > > > > > Reviewed-by: Alexandra Sandulescu <aesa@xxxxxxxxxx>
> > > > > > Reviewed-by: Jim Mattson <jmattson@xxxxxxxxxx>
> > > > > > Fixes: 7c693f54c873 ("x86/speculation: Add spectre_v2=ibrs option to support Kernel IBRS")
> > > > > > Cc: stable@xxxxxxxxxxxxxxx
> > > > > > Signed-off-by: KP Singh <kpsingh@xxxxxxxxxx>
> > > > > > ---
> > > > > >  arch/x86/kernel/cpu/bugs.c | 25 +++++++++++++++++--------
> > > > > >  1 file changed, 17 insertions(+), 8 deletions(-)
> > > > >
> > > > > As this is posted publicly, there's no need to send it to
> > > > > security@xxxxxxxxxx, it doesn't need to be involved.
> > > >
> > > > Sure, it's okay. Please do note in my first patch, I did follow
> > > > https://www.kernel.org/doc/Documentation/admin-guide/security-bugs.rst,
> > > > if you want folks to explicitly Cc maintainers with their fix or
> > > > report, I think it's worth mentioning in the guidelines there as the
> > > > current language seems to imply that the maintainers will be pulled in
> > > > by the security team:
> > > >
> > > > "It is possible that the security team will bring in extra help from
> > > > area maintainers to understand and fix the security vulnerability."
> > >
> > > Yes, but you already have a patch here, what "help" do you need?  You
> > > didn't specify any help, you just sent us a patch with no context.  This
> > > wasn't any sort of a "report" or "hey, I think we found a problem over
> > > here, does this change look correct", right?
> > >
> > > So please be specific as to what you are asking for, otherwise we have
> > > to guess (i.e. you cc:ed a seemingly random set of people but not the
> >
> > I don't see how it matters who I cc on the list.
>
> It gives us a hint as to who you are leaving out, right?
>
> > Anyways, I am still
> > not clear on what one is supposed to do in the case when one has a
> > patch for an issue already. Should this not be send it to security@?
>
> security@ is to take reports of potential security problems, triage
> them, and drag in the respective people to fix the problem as soon as
> possible by creating a patch and getting it merged.
>
> You already had a patch, so you did all of the work that security@ would
> normally do, so what did you need us to do here?
>
> You also did not ask or request anything, you just sent a patch with no
> context other than the changelog text.
>
> So if you have a fix for a potential problem already, you either just
> send it to get it merged as soon as possible, in which case security@ is
> not needed.  Or if you want to ask questions "is this really an issue
> and is this the fix", then send the patch and ask that question.
>
> Again, as it is, this looks like any other normal patch sent to
> subsystems for review, and there was no request for help or context at
> all.  Then you sent the patch to a public mailing list, so security@ is
> not needed at al, the normal development process applies as you
> determined it's not a secret by doing so.
>
> If you have questions, ask them, we can't read minds.

When one sends a report with the fix to security@xxxxxxxxxx, the ask /
request is they are asking for a review and having it merged. Now, I
thought this is implied with the patch, but maybe something else is
expected and I should have added a cover letter specifically asking
what was expected.

In this case, I did not intend to make it public initially and would
have preferred to not make it public until it was merged, until I was
nudged to read https://people.kernel.org/tglx/notes-about-netiquette.
Anyways, what's done is done, let's focus on fixing the bug.

>
> thanks,
>
> greg k-h




[Index of Archives]     [Linux Kernel]     [Kernel Development Newbies]     [Linux USB Devel]     [Video for Linux]     [Linux Audio Users]     [Yosemite Hiking]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux