linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Luck, Tony" <tony.luck@intel.com>
To: Borislav Petkov <bp@alien8.de>
Cc: Jue Wang <juew@google.com>, Ding Hui <dinghui@sangfor.com.cn>,
	naoya.horiguchi@nec.com, osalvador@suse.de,
	Youquan Song <youquan.song@intel.com>,
	huangcun@sangfor.com.cn, x86@kernel.org,
	linux-edac@vger.kernel.org, linux-mm@kvack.org,
	linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 1/3] x86/mce: Avoid infinite loop for copy from user recovery
Date: Fri, 20 Aug 2021 13:33:56 -0700	[thread overview]
Message-ID: <20210820203356.GA1623896@agluck-desk2.amr.corp.intel.com> (raw)
In-Reply-To: <YSACMCEoU6FxjDNh@zn.tnic>

On Fri, Aug 20, 2021 at 09:27:44PM +0200, Borislav Petkov wrote:
> On Fri, Aug 20, 2021 at 11:59:45AM -0700, Luck, Tony wrote:
> As in: there was an MCE while trying to access this user memory, you
> should not do get_user anymore. You did add that
> 
>          * Return zero to pretend that this copy succeeded. This
>          * is counter-intuitive, but needed to prevent the code
>          * in lib/iov_iter.c from retrying and running back into
> 
> which you're removing with the last patch so I'm confused.

Forget to address this part in the earlier reply.

My original code that forced a zero return has a hack. It
allowed recovery to complete, but only because there was
going to be a SIGBUS.  There were some unplesant side effects.
E.g. on a write syscall the file size was updated as if the
write had succeeded. That would be very confusing for anyone
trying to clean up afterwards as the file would have good
data that was copied from the user up to the point where
the machine check interrupted things. Then NUL bytes after
(because the kernel clears pages that are allocated into
the page cache).

The new version (thanks to All fixing iov_iter.c) now does
exactly what POSIX says should happen.  If I have a buffer
with poison at offset 213, and I do this:

	ret = write(fd, buf, 512);

Then the return from write is 213, and the first 213 bytes
from the buffer appear in the file, and the file size is
incremented by 213 (assuming the write started with the lseek
offset at the original size of the file).

-Tony


  parent reply	other threads:[~2021-08-20 20:34 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <20210706190620.1290391-1-tony.luck@intel.com>
2021-08-18  0:29 ` [PATCH v2 0/3] More machine check recovery fixes Tony Luck
2021-08-18  0:29   ` [PATCH v2 1/3] x86/mce: Avoid infinite loop for copy from user recovery Tony Luck
2021-08-20 17:31     ` Borislav Petkov
2021-08-20 18:59       ` Luck, Tony
2021-08-20 19:27         ` Borislav Petkov
2021-08-20 20:23           ` Luck, Tony
2021-08-21  4:51             ` Tony Luck
2021-08-21 21:51               ` Al Viro
2021-08-22 14:36             ` Borislav Petkov
2021-08-20 20:33           ` Luck, Tony [this message]
2021-08-22 14:46             ` Borislav Petkov
2021-08-23 15:24               ` Luck, Tony
2021-09-13  9:24     ` Borislav Petkov
2021-09-13 21:52       ` [PATCH v3] " Luck, Tony
2021-09-14  8:28         ` Borislav Petkov
2021-08-18  0:29   ` [PATCH v2 2/3] x86/mce: Change to not send SIGBUS error during copy from user Tony Luck
2021-08-18  0:29   ` [PATCH v2 3/3] x86/mce: Drop copyin special case for #MC Tony Luck
2021-09-20  9:13     ` Borislav Petkov
2021-09-20 16:18       ` Luck, Tony
2021-09-20 16:37         ` Borislav Petkov
2021-09-20 16:43           ` Luck, Tony
2021-08-18 16:14   ` [PATCH v2 0/3] More machine check recovery fixes Luck, Tony
2021-01-08 22:22 [PATCH 0/2] Fix infinite machine check loop in futex_wait_setup() Tony Luck
2021-01-11 21:44 ` [PATCH v2 0/3] " Tony Luck
2021-01-11 21:44   ` [PATCH v2 1/3] x86/mce: Avoid infinite loop for copy from user recovery Tony Luck
2021-01-11 22:11     ` Andy Lutomirski
2021-01-11 22:20       ` Luck, Tony
2021-01-12 17:00         ` Andy Lutomirski
2021-01-12 17:16           ` Luck, Tony
2021-01-12 17:21             ` Andy Lutomirski
2021-01-12 18:23               ` Luck, Tony
2021-01-12 18:57                 ` Andy Lutomirski
2021-01-12 20:52                   ` Luck, Tony
2021-01-12 22:04                     ` Andy Lutomirski
2021-01-13  1:50                       ` Luck, Tony
2021-01-13  4:15                         ` Andy Lutomirski
2021-01-13 10:00                           ` Borislav Petkov
2021-01-13 16:06                             ` Luck, Tony
2021-01-13 16:19                               ` Borislav Petkov
2021-01-13 16:32                                 ` Luck, Tony
2021-01-13 17:35                                   ` Borislav Petkov
2021-01-14 20:22     ` Borislav Petkov
2021-01-14 21:05       ` 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=20210820203356.GA1623896@agluck-desk2.amr.corp.intel.com \
    --to=tony.luck@intel.com \
    --cc=bp@alien8.de \
    --cc=dinghui@sangfor.com.cn \
    --cc=huangcun@sangfor.com.cn \
    --cc=juew@google.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=naoya.horiguchi@nec.com \
    --cc=osalvador@suse.de \
    --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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).