[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Thu, 27 Oct 2016 08:45:33 -0400
From: David Windsor <dave@...gbits.org>
To: Mark Rutland <mark.rutland@....com>
Cc: kernel-hardening@...ts.openwall.com,
"Reshetova, Elena" <elena.reshetova@...el.com>, AKASHI Takahiro <takahiro.akashi@...aro.org>,
Kees Cook <keescook@...omium.org>, Hans Liljestrand <ishkamiel@...il.com>,
Colin Vidal <colin@...dal.org>
Subject: Re: [RFC 0/2] arm: implementation of HARDENED_ATOMIC
Hi Mark,
On Thu, Oct 27, 2016 at 6:32 AM, Mark Rutland <mark.rutland@....com> wrote:
> Hi,
>
> On Tue, Oct 18, 2016 at 04:59:19PM +0200, Colin Vidal wrote:
>> Hi,
>>
>> This is the first attempt of HARDENED_ATOMIC port to arm arch.
>
> As a general note, please Cc relevant lists and people, as per
> get_maintainer.pl. For these patches that should tell you to Cc
> linux-kernel@...r.kernel.org, linux-arm-kernel@...ts.infradead.org, and
> a number of people familiar with the atomics.
>
> Even if things are far from perfect, and people don't reply (or reply
> but not too kindly), having them on Cc earlier makes it far more likely
> that issues are spotted and addressed earlier, minimizes repeatedly
> discussing the same issues, and also minimizes the potential for future
> arguments about these things being developed in isolation.
>
> Unless you do that, critical review for core code and arch code will
> come very late, and that could potentially delay this being merged for a
> very long time, which would be unfortunate.
>
>> About the fault handling I have some questions (perhaps some arm
>> expert are reading?):
>>
>> - As the process that made the overflow is killed, the kernel will
>> not try to go to a fixup address when the exception is raised,
>> right ? Therefore, is still mandatory to add an entry in the
>> __extable section?
>>
>> - In do_PrefetchAbort, I am unsure the code that follow the call to
>> hardened_atomic_overflow is needed: the process will be killed
>> anyways.
>
> Unfortunately, I'm only somewhat familiar with the ARM atomics, and I
> have absolutely no familiarity with the existing PaX patchset.
>
> For both of these, some background rationale would be helpful. e.g. what
> does the fixup entry do? When is it invoked?
>
For your reference, documentation on the original PaX protection
(known there a PAX_REFCOUNT) can be found here:
https://forums.grsecurity.net/viewtopic.php?f=7&t=4173
With respect to documentation, there is a patch in this series that
adds Documentation/security/hardened-atomic.txt, which references the
above-mentioned forum post.
Although, for long-term maintenance, maybe forum posts aren't the most
reliable thing in the world...
> I'll see what I can reverse-engineer from the patches.
>
>> I take some freedom compared to PaX patch, especially by adding some
>> macro to expand functions in arm/include/asm/atomic.h.
>>
>> The first patch is the modification I have done is generic part to
>> make it work.
>
> If you're relying on a prior patch series, please refer to that in the
> cover, to make it possible for reviewers to find.
>
> If you have a public git repo, placing this in a branch (or a tagged
> commit), and referring to that in the cover messages would make it much
> easier for people to review and/or test.
>
> Thanks,
> Mark.
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
