LKML Archive on
help / color / mirror / Atom feed
From: Dave Hansen <>
To: Ingo Molnar <>,
	Dave Hansen <>
Subject: Re: [PATCH 0/9] [v3] x86, pkeys: two protection keys bug fixes
Date: Tue, 8 May 2018 15:49:39 -0700	[thread overview]
Message-ID: <> (raw)
In-Reply-To: <>

> 1)
> Minor patch series organization requests:
>  - please include the shortlog and diffstat in the cover letter in the future, as 
>    it makes it easier to see the overall structure and makes it easier to reply to 
>    certain commits as a group.

Will do.

>  - please capitalize commit titles as is usually done in arch/x86/ and change the 
>    change the subsystem tags to the usual ones:
> d76eeb1914c8: x86/pkeys: Override pkey when moving away from PROT_EXEC
> f30f10248200: x86/pkeys/selftests: Add PROT_EXEC test
> 0530ebfefcdc: x86/pkeys/selftests: Add allow faults on unknown keys
> e81c40e33818: x86/pkeys/selftests: Factor out "instruction page"
> 57042882631c: x86/pkeys/selftests: Fix pkey exhaustion test off-by-one
> 6b833e9d3171: x86/pkeys/selftests: Fix pointer math
> d16f12e3c4ca: x86/pkeys: Do not special case protection key 0
> 1cb7691d0ee4: x86/pkeys/selftests: Add a test for pkey 0
> 273ae5cde423: x86/pkeys/selftests: Save off 'prot' for allocations
>  - please re-order the series to first introduce a unit test which specifically 
>    tests for the failure, ascertain that it indeed fails, and then apply the 
>    kernel fix. I.e. please use the order I used above for future versions of this 
>    patch-set.

I can't _quite_ use this order, but I get your point and I'll do as you
suggest, conceptually.

> 2)
> The new self-test you added does not fail overly nicely, it does the following on 
> older kernels:
> I.e. x86 unit tests should never 'crash' in a way that suggests that the testing 
> itself might be buggy - the crashes/failures should always be well controlled.

I've tried to make this nicer.  I never abort() any more, for instance.

> 3)
> When the first kernel bug fix is applied but not the second, then I don't see the 
> new PROT_EXEC test catching the bug:

Thanks for catching this.  I forgot to add the test function to the
pkey_tests[] array.  It's fixed up now.

> 4)
> In the above kernel that was missing the PROT_EXEC fix I was repeatedly running 
> the 64-bit and 32-bit testcases as non-root and as root as well, until I got a 
> hang in the middle of a 32-bit test running as root:

I believe this is all my stupidity from not being careful about using
signal-safe functions in the signal handlers.  There's no pretty
solution for this, but I've at least made it stop hanging.  The fixes
for that will be in the beginning of the next series.

      parent reply	other threads:[~2018-05-08 22:49 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-04-27 17:45 Dave Hansen
2018-04-27 17:45 ` [PATCH 1/9] x86, pkeys: do not special case protection key 0 Dave Hansen
2018-04-27 17:45 ` [PATCH 2/9] x86, pkeys, selftests: save off 'prot' for allocations Dave Hansen
2018-04-27 17:45 ` [PATCH 3/9] x86, pkeys, selftests: add a test for pkey 0 Dave Hansen
2018-04-27 17:45 ` [PATCH 4/9] x86, pkeys: override pkey when moving away from PROT_EXEC Dave Hansen
2018-04-27 17:45 ` [PATCH 5/9] x86, pkeys, selftests: fix pointer math Dave Hansen
2018-04-27 17:45 ` [PATCH 6/9] x86, pkeys, selftests: fix pkey exhaustion test off-by-one Dave Hansen
2018-04-27 17:45 ` [PATCH 7/9] x86, pkeys, selftests: factor out "instruction page" Dave Hansen
2018-04-27 17:45 ` [PATCH 8/9] x86, pkeys, selftests: add allow faults on unknown keys Dave Hansen
2018-04-27 17:45 ` [PATCH 9/9] x86, pkeys, selftests: add PROT_EXEC test Dave Hansen
2018-04-28  7:05 ` [PATCH 0/9] [v3] x86, pkeys: two protection keys bug fixes Ingo Molnar
2018-04-28  7:15   ` Ingo Molnar
2018-04-28  8:29     ` Ingo Molnar
2018-04-30 15:30   ` Dave Hansen
2018-04-30 16:28     ` Ram Pai
2018-05-08 22:49   ` Dave Hansen [this message]

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:

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \ \ \ \ \ \ \ \ \ \ \ \ \
    --subject='Re: [PATCH 0/9] [v3] x86, pkeys: two protection keys bug fixes' \

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

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