Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1403364 > unrolled thread

[PATCH] x86, signals: add missing signal_compat code for x86 features

Back to article view | Back to linux.kernel

Contents

  [PATCH] x86, signals: add missing signal_compat code for x86 features Dave Hansen <dave@sr71.net> - 2016-05-19 02:00 +0200
    Re: [PATCH] x86, signals: add missing signal_compat code for x86  features Ingo Molnar <mingo@kernel.org> - 2016-05-20 09:10 +0200
      Re: [PATCH] x86, signals: add missing signal_compat code for x86  features Dave Hansen <dave@sr71.net> - 2016-05-25 00:40 +0200
        RE: [PATCH] x86, signals: add missing signal_compat code for x86  features "Luck, Tony" <tony.luck@intel.com> - 2016-05-25 00:50 +0200

#1403364 — [PATCH] x86, signals: add missing signal_compat code for x86 features

Sending this out early so folks can have a look.  I haven't let
it run through a full set of tests, so buyer beware, but it would
have a hard time hurting anything other than the already-broken
32-bit compat signal code.

---

From: Dave Hansen <dave.hansen@linux.intel.com>

The 32-bit siginfo is a different binary format than the 64-bit
one.  So, when running 32-bit binaries on 64-bit kernels, we have
to convert the kernel's 64-bit version to a 32-bit version that
userspace can grok.

We've added a few features to siginfo over the past few years and
neglected to add them to arch/x86/kernel/signal_compat.c:

   1. The si_addr_lsb used in SIGBUS's sent for machine checks
   2. The upper/lower bounds for MPX SIGSEGV faults
   3. The protection key for pkey faults

I caught this with some protection keys unit tests and realized
it affected a few more features.

This was tested only with my protection keys patch that looks
for a proper value in si_pkey.  I didn't actually test the machine
check or MPX code.

Signed-off-by: Dave Hansen <dave.hansen@linux.intel.com>
Cc: Tony Luck <tony.luck@intel.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: linux-edac@vger.kernel.org
Cc: x86@kernel.org
Cc: Andy Lutomirski <luto@kernel.org>
---

 b/arch/x86/include/asm/compat.h   |   11 +++++++++++
 b/arch/x86/kernel/signal_compat.c |   13 +++++++++++++
 2 files changed, 24 insertions(+)

diff -puN arch/x86/include/asm/compat.h~add-signal-compat-for-mpx-pkeys arch/x86/include/asm/compat.h
--- a/arch/x86/include/asm/compat.h~add-signal-compat-for-mpx-pkeys	2016-05-18 15:29:27.238736544 -0700
+++ b/arch/x86/include/asm/compat.h	2016-05-18 15:29:27.243736772 -0700
@@ -40,6 +40,7 @@ typedef s32		compat_long_t;
 typedef s64 __attribute__((aligned(4))) compat_s64;
 typedef u32		compat_uint_t;
 typedef u32		compat_ulong_t;
+typedef u32		compat_u32;
 typedef u64 __attribute__((aligned(4))) compat_u64;
 typedef u32		compat_uptr_t;
 
@@ -181,6 +182,16 @@ typedef struct compat_siginfo {
 		/* SIGILL, SIGFPE, SIGSEGV, SIGBUS */
 		struct {
 			unsigned int _addr;	/* faulting insn/memory ref. */
+			short int _addr_lsb;	/* Valid LSB of the reported address. */
+			union {
+				/* used when si_code=SEGV_BNDERR */
+				struct {
+					compat_uptr_t _lower;
+					compat_uptr_t _upper;
+				} _addr_bnd;
+				/* used when si_code=SEGV_PKUERR */
+				compat_u32 _pkey;
+			};
 		} _sigfault;
 
 		/* SIGPOLL */
diff -puN arch/x86/kernel/signal_compat.c~add-signal-compat-for-mpx-pkeys arch/x86/kernel/signal_compat.c
--- a/arch/x86/kernel/signal_compat.c~add-signal-compat-for-mpx-pkeys	2016-05-18 15:29:27.240736635 -0700
+++ b/arch/x86/kernel/signal_compat.c	2016-05-18 16:54:54.366568908 -0700
@@ -32,6 +32,19 @@ int copy_siginfo_to_user32(compat_siginf
 					  &to->_sifields._pad[0]);
 			switch (from->si_code >> 16) {
 			case __SI_FAULT >> 16:
+		                if (from->si_signo == SIGBUS &&
+		                    (from->si_code == BUS_MCEERR_AR ||
+				     from->si_code == BUS_MCEERR_AO))
+					put_user_ex(from->si_addr_lsb, &to->si_addr_lsb);
+
+				if (from->si_signo == SIGSEGV) {
+				       	if (from->si_code == SEGV_BNDERR) {
+						put_user_ex(from->si_lower, &to->si_lower);
+						put_user_ex(from->si_upper, &to->si_upper);
+					}
+				       	if (from->si_code == SEGV_PKUERR)
+						put_user_ex(from->si_pkey, &to->si_pkey);
+				}
 				break;
 			case __SI_SYS >> 16:
 				put_user_ex(from->si_syscall, &to->si_syscall);
_

[toc] | [next] | [standalone]

#1404182 — Re: [PATCH] x86, signals: add missing signal_compat code for x86 features

* Dave Hansen <dave@sr71.net> wrote:

> Sending this out early so folks can have a look.  I haven't let
> it run through a full set of tests, so buyer beware, but it would
> have a hard time hurting anything other than the already-broken
> 32-bit compat signal code.
> 
> ---
> 
> From: Dave Hansen <dave.hansen@linux.intel.com>
> 
> The 32-bit siginfo is a different binary format than the 64-bit
> one.  So, when running 32-bit binaries on 64-bit kernels, we have
> to convert the kernel's 64-bit version to a 32-bit version that
> userspace can grok.
> 
> We've added a few features to siginfo over the past few years and
> neglected to add them to arch/x86/kernel/signal_compat.c:
> 
>    1. The si_addr_lsb used in SIGBUS's sent for machine checks
>    2. The upper/lower bounds for MPX SIGSEGV faults
>    3. The protection key for pkey faults
> 
> I caught this with some protection keys unit tests and realized
> it affected a few more features.

Hm, while fixing this, could we please also add individual unit tests to 
tools/testing/selftests/x86/, and also structure the code in a fashion or add a 
comment or so to make sure future extensions add both a compat handler and a unit 
test as well?

I.e. perhaps do a (build time) fixed-size check of siginfo structure in the compat 
code, and break the build if that check has not been updated? Or something like 
that.

Thanks,

	Ingo

[toc] | [prev] | [next] | [standalone]

#1406487 — Re: [PATCH] x86, signals: add missing signal_compat code for x86 features

On 05/20/2016 12:05 AM, Ingo Molnar wrote:
>> We've added a few features to siginfo over the past few years and
>> neglected to add them to arch/x86/kernel/signal_compat.c:
>>
>>    1. The si_addr_lsb used in SIGBUS's sent for machine checks
>>    2. The upper/lower bounds for MPX SIGSEGV faults
>>    3. The protection key for pkey faults
>>
>> I caught this with some protection keys unit tests and realized
>> it affected a few more features.
> 
> Hm, while fixing this, could we please also add individual unit tests to 
> tools/testing/selftests/x86/, and also structure the code in a fashion or add a 
> comment or so to make sure future extensions add both a compat handler and a unit 
> test as well?

The test that found this was one of the tests I'm submitting with
protection keys, so that's covered.  I also improved my out-of-tree MPX
tests to cover this too.  I can submit a version of those to be kept
in-tree.

Tony / Borislav, do we have tests for the machine check code that could
have caught this?

> I.e. perhaps do a (build time) fixed-size check of siginfo structure in the compat 
> code, and break the build if that check has not been updated? Or something like 
> that.

A size check of all the individual parts of the structure would help.
But it won't be good enough to catch everything.  Protection keys, for
example, piggybacked on the space that MPX already carved out.  So a
size check would not have caught it.

I'll probably go the heavy commenting route, along with some size checks
to _help_.

[toc] | [prev] | [next] | [standalone]

#1406493 — RE: [PATCH] x86, signals: add missing signal_compat code for x86 features

> Tony / Borislav, do we have tests for the machine check code that could
> have caught this?

If I had built one of my recovery test programs as a 32-byte binary instead of native 64-bit I might have noticed (I only print the lsb field ... which would have been garbage on the stack, maybe I'd have spotted a silly value).

-Tony

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web