All of lore.kernel.org
 help / color / mirror / Atom feed
From: Borislav Petkov <bp@alien8.de>
To: "Luck, Tony" <tony.luck@intel.com>
Cc: Youquan Song <youquan.song@intel.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 5/8] x86/mce: Avoid tail copy when machine check terminated a copy from user
Date: Thu, 17 Sep 2020 19:04:06 +0200	[thread overview]
Message-ID: <20200917170406.GN31960@zn.tnic> (raw)
In-Reply-To: <20200916192659.GA30252@agluck-desk2.amr.corp.intel.com>

Replying a bit out of order:

> Both the code and the commit comment need much more of this
> description.

Hell yeah!

On Wed, Sep 16, 2020 at 12:26:59PM -0700, Luck, Tony wrote:
> So we take a another machine check on the same address when
> fault_in_pages_readable() does __get_user().  That ought to break
> us out ... but for some reason I still don't understand didn't.
> But even if it did ... the second machine check is not at all
> a good idea.

And I think this is the important point: for MCEs you absolutely don't
want to take another MCE and even walk into those fields. So what
fault_in_pages_readable() does normally, would be totally wrong. Imagine
you're playing minesweeper - you can't just pre-fault blocks without
counting the mines. :-P

So actually, I'm thinking:

.LMCE_during_user_access:
	mov $-ENODEV, %eax
	ASM_CLAC
	ret

I have no clue which error code we should put there but it should be an
error code which tells you not to retry and to back off immediately.

> Returning zero bytes left to say we completed avoids that. The
> user is guaranteed a SIGBUS when the task_work does fire. So whatever
> system call was in progress is not going to see the apparent
> successful return.

Yes, my only proposition with the error code is in case you're looking
at traces, to recognize that the copying encountered an MCE. In addition
to the "back off immediately" semantics, if there even is such defined
for users of copy_*_user().

> Unless you have some better way out of the dilemmma that the
> real fixup is only scheduled at the point that the extable
> fixup just arranges for a simple local return from the copy.

Right, see above: I think it is imperative *not* to walk into that area
again and not do any retrying.

> When the return to user happens the task_work that was scheduled
> in the machine check handler takes care of the error return to the
> user.

Yeah, let's write that whole flow down somewhere - not in a commit
message - so that it is clear what happens.

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

  reply	other threads:[~2020-09-17 17:05 UTC|newest]

Thread overview: 49+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20200908175519.14223-1-tony.luck@intel.com>
2020-09-08 17:55 ` [PATCH 1/8] x86/mce: Stop mce_reign() from re-computing severity for every CPU Tony Luck
2020-09-14 17:21   ` Borislav Petkov
2020-09-14 17:32   ` [tip: ras/core] " tip-bot2 for Tony Luck
2020-09-08 17:55 ` [PATCH 4/8] x86/mce: Add _ASM_EXTABLE_CPY for copy user access Tony Luck
2020-09-16  9:59   ` Borislav Petkov
2020-09-08 17:55 ` [PATCH 5/8] x86/mce: Avoid tail copy when machine check terminated a copy from user Tony Luck
2020-09-16 10:53   ` Borislav Petkov
2020-09-16 19:26     ` Luck, Tony
2020-09-17 17:04       ` Borislav Petkov [this message]
2020-09-17 21:57         ` Luck, Tony
2020-09-18  7:51           ` Borislav Petkov
2020-09-08 17:55 ` [PATCH 6/8] x86/mce: Change fault_in_kernel_space() from static to global Tony Luck
2020-09-08 17:55 ` [PATCH 7/8] x86/mce: Recover from poison found while copying from user space Tony Luck
2020-09-18 16:13   ` Borislav Petkov
2020-09-08 17:55 ` [PATCH 8/8] x86/mce: Decode a kernel instruction to determine if it is copying from user Tony Luck
2020-09-21 11:31   ` Borislav Petkov
2020-09-30 23:26     ` [PATCH v2 0/7] Add machine check recovery when copying from user space Tony Luck
2020-09-30 23:26       ` [PATCH v2 1/7] x86/mce: Pass pointer to saved pt_regs to severity calculation routines Tony Luck
2020-09-30 23:26       ` [PATCH v2 2/7] x86/mce: Provide method to find out the type of exception handle Tony Luck
2020-10-05 16:35         ` Borislav Petkov
2020-09-30 23:26       ` [PATCH v2 3/7] x86/mce: Add _ASM_EXTABLE_CPY for copy user access Tony Luck
2020-10-05 16:34         ` Borislav Petkov
2020-09-30 23:26       ` [PATCH v2 4/7] x86/mce: Avoid tail copy when machine check terminated a copy from user Tony Luck
2020-09-30 23:26       ` [PATCH v2 5/7] x86/mce: Change fault_in_kernel_space() from static to global Tony Luck
2020-10-05 16:33         ` Borislav Petkov
2020-09-30 23:26       ` [PATCH v2 6/7] x86/mce: Recover from poison found while copying from user space Tony Luck
2020-10-05 16:32         ` Borislav Petkov
2020-10-05 17:47           ` Luck, Tony
2020-09-30 23:26       ` [PATCH v2 7/7] x86/mce: Decode a kernel instruction to determine if it is copying from user Tony Luck
2020-10-05 16:31         ` Borislav Petkov
2020-10-06 21:09           ` [PATCH v3 0/6] Add machine check recovery when copying from user space Tony Luck
2020-10-06 21:09             ` [PATCH v3 1/6] x86/mce: Pass pointer to saved pt_regs to severity calculation routines Tony Luck
2020-10-07 10:02               ` [tip: ras/core] " tip-bot2 for Youquan Song
2020-10-06 21:09             ` [PATCH v3 2/6] x86/mce: Provide method to find out the type of exception handle Tony Luck
2020-10-07 10:02               ` [tip: ras/core] x86/mce: Provide method to find out the type of an exception handler tip-bot2 for Tony Luck
2020-10-06 21:09             ` [PATCH v3 3/6] x86/mce: Add _ASM_EXTABLE_CPY for copy user access Tony Luck
2020-10-07 10:02               ` [tip: ras/core] " tip-bot2 for Youquan Song
2020-10-06 21:09             ` [PATCH v3 4/6] x86/mce: Avoid tail copy when machine check terminated a copy from user Tony Luck
2020-10-07  8:23               ` David Laight
2020-10-07 18:49                 ` Luck, Tony
2020-10-07 21:11                   ` David Laight
2020-10-07 10:02               ` [tip: ras/core] " tip-bot2 for Tony Luck
2020-10-06 21:09             ` [PATCH v3 5/6] x86/mce: Recover from poison found while copying from user space Tony Luck
2020-10-07 10:02               ` [tip: ras/core] " tip-bot2 for Tony Luck
2020-10-06 21:09             ` [PATCH v3 6/6] x86/mce: Decode a kernel instruction to determine if it is copying from user Tony Luck
2020-10-07 10:02               ` [tip: ras/core] " tip-bot2 for Tony Luck
2020-09-09 15:05 ` [RESEND PATCH 0/8] Add machine check recovery when copying from user space Tony Luck
     [not found] ` <20200908175519.14223-4-tony.luck@intel.com>
2020-09-15  9:11   ` [PATCH 3/8] x86/mce: Provide method to find out the type of exception handle Borislav Petkov
2020-09-15 16:24     ` Luck, Tony

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=20200917170406.GN31960@zn.tnic \
    --to=bp@alien8.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    --cc=youquan.song@intel.com \
    /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: link
Be 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.