public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Luck, Tony" <tony.luck@intel.com>
To: David Laight <David.Laight@ACULAB.COM>, Borislav Petkov <bp@alien8.de>
Cc: "Song, Youquan" <youquan.song@intel.com>,
	"x86@kernel.org" <x86@kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Subject: RE: [PATCH v3 4/6] x86/mce: Avoid tail copy when machine check terminated a copy from user
Date: Wed, 7 Oct 2020 18:49:51 +0000	[thread overview]
Message-ID: <c0791688c58f434ca9e413630504d22d@intel.com> (raw)
In-Reply-To: <c994091cdc9d42718769f584b7d4a134@AcuMS.aculab.com>

>> Machine checks are more serious. Just give up at the point where the
>> main copy loop triggered the #MC and return from the copy code as if
>> the copy succeeded. The machine check handler will use task_work_add() to
>> make sure that the task is sent a SIGBUS.
>
> Isn't that just plain wrong?

It isn't pretty. I'm not sure how wrong it is.

> If copy is reported as succeeding the kernel code will use the 'old'
> data that is in the buffer as if it had been read from userspace.
> This could end up with kernel stack data being written to a file.

I ran a test with:

	write(fd, buf, 512)

With poison injected into buf[256] to force a machine check mid-copy.

The size of the file did get incremented by 512 rather than 256. Which isn't good.

The data in the file up to the 256 byte mark was the user data from buf[0 ... 255].

The data in the file past offset 256 was all zeroes. I suspect that isn't by chance.
The kernel has to defend against a user writing a partial page and using mmap(2)
on the same file to peek at data past EOF and up to the next PAGE_SIZE boundary.
So I think it must zero new pages allocated in page cache as they are allocated to
a file.

> Even zeroing the rest of the kernel buffer is wrong.

It wouldn't help/change anything.

> IIRC the code to try to maximise the copy has been removed.
> So the 'slow' retry wont happen any more.

Which code has been removed (and when ... TIP, and my testing, is based on 5.9-rc1)

-Tony


  reply	other threads:[~2020-10-07 19:24 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
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 [this message]
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=c0791688c58f434ca9e413630504d22d@intel.com \
    --to=tony.luck@intel.com \
    --cc=David.Laight@ACULAB.COM \
    --cc=bp@alien8.de \
    --cc=linux-kernel@vger.kernel.org \
    --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