Re: [PATCH 3/3] powerpc: enable support for GCC plugins

[PaX Team]

On 9 Dec 2016 at 13:48, Andrew Donnellan wrote:

> >> as for the solutions, the general advice should enable the use of otherwise
> >> failing gcc versions instead of forcing updating to new ones (though the
> >> latter is advisable for other reasons but not everyone's in the position to
> >> do so easily). in my experience all one needs to do is manually install the
> >> missing files from the gcc sources (ideally distros would take care of it).
> 
> If someone else is willing to write up that advice, then great.
> 
> >> the specific problem addressed here can (and IMHO should) be solved in
> >> another way: remove the inclusion of the offending headers in gcc-common.h
> >> as neither tm.h nor c-common.h are needed by existing plugins. for 
> >> background,
> 
> We can't build without tm.h: http://pastebin.com/W0azfCr0

you'll need to repeat the removal of dependent headers. based on a quick
test here across gcc 4.5-6.2, if you remove rtl.h, tm_p.h, hard-reg-set.h
and emit-rtl.h in addition to tm.h, the plugins should build fine.

> And we get warnings without c-common.h: http://pastebin.com/Aw8CAj10

that's not due to c-common.h. gcc versions 4.5-4.6 are compiled as a C program
and gcc 4.7 can be compiled both as a C and a C++ program (IIRC, distros opted
for the latter, i forget what manually built versions default to but i guess you
went with the C compilation for your gcc anyway). couple that with 
-Wmissing-prototypes
and you get that warning regardless of c-common.h being included. something like
this should fix it:

--- a/scripts/gcc-plugins/gcc-generate-gimple-pass.h 2016-12-06 
01:01:54.521724573 +0100
+++ b/scripts/gcc-plugins/gcc-generate-gimple-pass.h  2016-12-09 
11:43:32.225226164 +0100
@@ -136,6 +136,7 @@
return new _PASS_NAME_PASS();
 }
 #else
+struct opt_pass *_MAKE_PASS_NAME_PASS(void);
 struct opt_pass *_MAKE_PASS_NAME_PASS(void)
 {
return &_PASS_NAME_PASS.pass;

> These were all manually built using a script running on a Debian box. 
> Installing precompiled distro versions of rather old gccs would have 
> been somewhat challenging. I've just rebuilt 4.6.4 to double check that 
> I wasn't just seeing things, but it seems that it definitely is still 
> putting c-common.h in the old location.

for reference, this is the git commit that did the move:

commit 7bedc3a05d34cd81e4835a2d3ff8c0ec7108eeb5
Author: steven 
Date:   Sat Jun 5 20:33:22 2010 +

gcc/ChangeLog:
* c-common.c: Move to c-family/.
* c-common.def: Likewise.
* c-common.h: Likewise.

Re: [PATCH 3/3] powerpc: enable support for GCC plugins

[PaX Team]

On 6 Dec 2016 at 17:28, Andrew Donnellan wrote:

> Enable support for GCC plugins on powerpc.
> 
> Add an additional version check in gcc-plugins-check to advise users to
> upgrade to gcc 5.2+ on powerpc to avoid issues with header files (gcc <=
> 4.6) or missing copies of rs6000-cpus.def (4.8 to 5.1 on 64-bit targets).

i don't think that this is the right approach. there's a general and a special
issue here, both of which need different handling.

the general problem is to detect problems related to gcc plugin headers and
notify the users about solutions. emitting various messages from a Makefile
is certainly not a scalable approach, just imagine how it will look when the
other 30+ archs begin to add their own special cases... if anything, they
should be documented in Documentation/gcc-plugins.txt (or a new doc if it
grows too big) and the Makefile message should just point at it.

as for the solutions, the general advice should enable the use of otherwise
failing gcc versions instead of forcing updating to new ones (though the
latter is advisable for other reasons but not everyone's in the position to
do so easily). in my experience all one needs to do is manually install the
missing files from the gcc sources (ideally distros would take care of it).

the specific problem addressed here can (and IMHO should) be solved in
another way: remove the inclusion of the offending headers in gcc-common.h
as neither tm.h nor c-common.h are needed by existing plugins. for background,
i created gcc-common.h to simplify plugin development across all supportable
gcc versions i came across over the years, so it follows the 'everything but
the kitchen sink' approach. that isn't necessarily what the kernel and other
projects need so they should just use my version as a basis and fork/simplify
it (even i maintain private forks of the public version).

as for the location of c-common.h, upstream gcc moved it under c-family in
2010 after the release of 4.5, so it should be where gcc-common.h expects
it and i'm not sure how it ended up at its old location for you.

cheers,
 PaX Team

Re: [PATCH 0/9] mm: Hardened usercopy

[PaX Team]

On 10 Jul 2016 at 11:16, Ingo Molnar wrote:

> * PaX Team <pagee...@freemail.hu> wrote:
> 
> > On 9 Jul 2016 at 14:27, Andy Lutomirski wrote:
> > 
> > > I like the series, but I have one minor nit to pick.  The effect of this 
> > > series is to harden usercopy, but most of the code is really about 
> > > infrastructure to validate that a pointed-to object is valid.
> > 
> > actually USERCOPY has never been about validating pointers. its sole 
> > purpose is 
> > to validate the *size* argument of copy*user calls, a very specific form of 
> > runtime bounds checking.
> 
> What this code has been about originally is largely immaterial, unless you 
> can 
> formulate it into a technical argument.

we design defense mechanisms for specific and clear purposes, starting with
a threat model, evaluating defense options based on various criteria, etc.
USERCOPY underwent this same process and taking it out of its original context
means that all you get in the end is cargo cult security (wouldn't be the first
time it has happened (ExecShield, ASLR, etc)).

that said, i actually started that discussion but for some reason you chose
not to respond to that one part of my mail so let me ask it again:

  what kind of checks are you thinking of here? and more fundamentally, against
  what kind of threats?

as far as i'm concerned, a defense mechanism is only as good as its underlying
threat model. by validating pointers (for yet to be stated security related
properties) you're presumably assuming some kind of threat and unless stated
clearly what that threat is (unintended pointer modification through memory
corruption and/or other bugs?) noone can tell whether the proposed defense
mechanism will actually be effective in preventing exploitation. it is the
worst kind of defense that doesn't actually achieve its stated goals, that
way lies false sense of security and i hope noone here is in that business.

i note that this analysis is also missing from this USERCOPY submission except
for stating what Kees assumed about USERCOPY (and apparently noone could be
bothered to read the original Kconfig help of it which clearly states that the
purpose is copy size checking, not some elaborate pointer validation, the latter
is an implementation detail only and is necessary to be able to derive the
underlying slab object's intended size).

> There are a number of cheap tests we can do and there are a number of ways 
> how a 
> 'pointer' can be validated runtime, without any 'size' information:
> 
>  - for example if a pointer points into a red zone straight away then we know 
> it's
>bogus.

it's not pointer validation but bounds checking: you already know which memory
object the pointer is supposed to point to, you only check its bounds. if it was
an attacker controlled pointer then all this would be a pointless check of 
course,
trivial for an attacker to circumvent (and this is why it's not part of the
USERCOPY design).

>  - or if a kernel pointer is points outside the valid kernel virtual memory 
> range
>we know it's bogus as well.

accesses outside of valid virtual memory will cause a page fault ('oops' in 
linux
terms), there's no need to explicitly check for that.

> So while only doing a bounds check might have been the original purpose of 
> the 
> patch set, Andy's point is that it might make sense to treat this facility as 
> a 
> more generic 'object validation' code of (pointer,size) object and not limit 
> it to 
> 'runtime bounds checking'.

FYI, 'runtime bounds checking' is a terminus technicus and it is about 
validating
both the pointer and underlying object's size. that's the reason i called 
USERCOPY
a 'very specific form' of it only since it doesn't validate each part equally 
well
(or well enough at all, even the size check is not as precise as it could be).

as for what does or doesn't make sense, first you'll have to define a threat
model and evaluate everything else based on that. since noone has solved the
general bounds checking problem with acceptable properties (mostly performance
impact, but also memory overhead, etc), i'm all ears to hear what you guys have
come up with.

> That kind of extended purpose behind a facility should be reflected in the 
> naming.
> Confusing names are often the source of misunderstandings and bugs.

definitely, but before you bikeshed on naming, you should figure out what and 
why
you want to do, whether it's even feasible, meaningful, useful, etc. answering 
the
opening question and digging into the details is the first step of any design
process, not its naming.

> The 9-patch series as submitted here is neither just 'bounds checking' nor 
> just 
> pure 'pointer checking', it's about validating that a (pointer,size) range of 
> memory passed to a (user) memory copy function is fully within a vali

Re: [PATCH 0/9] mm: Hardened usercopy

[PaX Team]

On 9 Jul 2016 at 14:27, Andy Lutomirski wrote:

> On Jul 6, 2016 6:25 PM, "Kees Cook"  wrote:
> >
> > Hi,
> >
> > This is a start of the mainline port of PAX_USERCOPY[1]. After I started
> > writing tests (now in lkdtm in -next) for Casey's earlier port[2], I
> > kept tweaking things further and further until I ended up with a whole
> > new patch series. To that end, I took Rik's feedback and made a number
> > of other changes and clean-ups as well.
> >
> 
> I like the series, but I have one minor nit to pick.  The effect of
> this series is to harden usercopy, but most of the code is really
> about infrastructure to validate that a pointed-to object is valid.

actually USERCOPY has never been about validating pointers. its sole purpose
is to validate the *size* argument of copy*user calls, a very specific form
of runtime bounds checking. it's only really relevant for slab objects and the
pointer checks (that one might mistake for being a part of the defense 
mechanism)
are only there to determine whether the kernel pointer refers to a slab object
or not (the stack part is a small bonus and was never the main goal either).

> Might it make sense to call the infrastructure part something else?

yes, more bikeshedding will surely help, like the renaming of .data..read_only
to .data..ro_after_init which also had nothing to do with init but everything
to do with objects being conceptually read-only...

> After all, this could be extended in the future for memcpy or even for
> some GCC plugin to check pointers passed to ordinary (non-allocator)
> functions.

what kind of checks are you thinking of here? and more fundamentally, against
what kind of threats? as for memcpy, it's the standard mandated memory copying
function, what security related properties can it check on its pointer 
arguments?

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev

Re: [kernel-hardening] [PATCH v8 2/4] GCC plugin infrastructure

[PaX Team]

On 19 May 2016 at 16:22, Michael Ellerman wrote:

> On Wed, 2016-05-18 at 12:33 +0200, Emese Revfy wrote:
> > Did you test the plugins with all gcc versions (4.5-6)?
> 
> What's the concern about gcc versions? Just not breaking the build on old
> compilers?

the earlier plugin capable gcc versions used to install gcc headers in a 
somewhat
ad-hoc manner resulting in compile time breakage for plugins and since some of
those potentially missing headers are target specific, each target arch should
be verified before enabling plugin support on them. things have much improved 
with
gcc 5 (see https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61176) though there's 
still
an occasional missing header but with wider use of plugins they will hopefully 
be
discovered earlier now. perhaps linux-arch should be cc'ed on the plugin 
infrastructure
so that arch maintainers are aware of this?

> I'm pretty sure powerpc big endian still builds with gcc 4.4.
> 
> However if Andrew's only tested on little endian, then that select should be
> guarded with an "if CPU_LITTLE_ENDIAN". And to build LE you need gcc >= 4.9.

i guess that's part of the target tuple so in general arch maintainers should 
test
the target tuples used on their arch with all the supported gcc versions 
(speaking
of CC, not HOSTCC/HOSTCXX).

cheers,
 PaX Team

___
Linuxppc-dev mailing list
Linuxppc-dev@lists.ozlabs.org
https://lists.ozlabs.org/listinfo/linuxppc-dev