[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Wed, 26 Oct 2016 10:44:49 +0200
From: Colin Vidal <colin@...dal.org>
To: kernel-hardening@...ts.openwall.com
Cc: Kees Cook <keescook@...omium.org>, David Windsor <dave@...gbits.org>,
"Reshetova, Elena" <elena.reshetova@...el.com>
Subject: Re: [RFC v2 PATCH 00/13] HARDENED_ATOMIC
Hi Elena,
> > Perhaps the cleaner solution would be to define prototypes (of real functions, not macros) of atomic64_*_wrap functions in asm- generic/atomic64.h. If the arch has its own implementation of atomic64, no problems (asm-generic/atomic64.h is not >included), and if
> > CONFIG_GENERIC_ATOMIC64 is set, we just need to implements atomic64_*_wrap functions (in a arch/lib/atomic64.c, for instance).
>
> Why not in lib/atomic64.c? Because this is what would be used in the case when CONFIG_GENERIC_ATOMIC is set. Also, when you say "implement", do you mean actually provide "protected" versions of wrap functions or just wrap functions themselves operating on wrap_t types but providing no difference from non-wrap functions?
>
The point is if CONFIG_GENERIC_ATOMIC64 is unset, asm-
generic/atomic64.h and lib/atomic64.c (see lib/Makefile) are not
included in compilation, therefore we don't care about that: the
architecture has its own implementation of atomic64 (protected and wrap
version).
If CONFIG_GENERIC_ATOMIC64 is set, the common implementation is in
lib/atomic64.c. Therefore, any functions in lib/atomic64.c should be
renamed to be suffixed by _wrap (unprotected versions), and prototypes
with _wrap suffix should be added in asm-generic/atomic64.h.
The protected versions of functions depends of each architecture,
therefore they can't be generic. That why I propose to add implements
them in arch/lib/atomic64.c (or even arch/lib/atomic64_wrap.c, it is
much clear).
And yes, by "implement" I means write "protected" versions. Err... Yes,
in that case, we don't have the "examples" implemented in PAX. But the
other solution would be leave GENERIC_ATOMIC64 unprotected, which is
really unclear from the "user" point-of-view.
If CONFIG_GENERIC_ATOMIC64 is set and CONFIG_HARDENED_ATOMIC is unset,
the implementations in arch/lib/atomic64_wrap.c just does not include
the overflow test on the add/sub operations, like the current protected
arm/x64 implementations.
> Overall, I agree, there is no magic, but IMO definitions are so confusing even without wrap added (local_t and its functions is a good example), that tracking them all down is a pain.
> At some point we used this table to track functions and their definition: https://docs.google.com/spreadsheets/d/12wX-csPY8tnHpQVRKVFPSOoV8o6WXtAwKKTAkx8uUIQ/edit?usp=sharing
> Maybe it can be extended to include more info that people would like to see there.
That would be useful and avoid numerous post-it on my desk :-) Thanks!
Best regards,
Colin
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.
