All of lore.kernel.org
 help / color / mirror / Atom feed
From: Dave Hansen <dave@sr71.net>
To: Ingo Molnar <mingo@kernel.org>
Cc: linux-kernel@vger.kernel.org, dave.hansen@linux.intel.com,
	tony.luck@intel.com, bp@alien8.de, linux-edac@vger.kernel.org,
	x86@kernel.org, luto@kernel.org
Subject: Re: [PATCH] x86, signals: add missing signal_compat code for x86 features
Date: Tue, 24 May 2016 15:31:35 -0700	[thread overview]
Message-ID: <5744D647.2000108@sr71.net> (raw)
In-Reply-To: <20160520070559.GA4003@gmail.com>

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_.

  reply	other threads:[~2016-05-24 22:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-05-18 23:57 [PATCH] x86, signals: add missing signal_compat code for x86 features Dave Hansen
2016-05-20  7:05 ` Ingo Molnar
2016-05-24 22:31   ` Dave Hansen [this message]
2016-05-24 22:49     ` 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=5744D647.2000108@sr71.net \
    --to=dave@sr71.net \
    --cc=bp@alien8.de \
    --cc=dave.hansen@linux.intel.com \
    --cc=linux-edac@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luto@kernel.org \
    --cc=mingo@kernel.org \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /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.