On 5 Jan 2012 at 17:21, Andrea Zuccherelli wrote:
> > now if some code needs writable ops structure variables it has 3 options under
> > the plugin approach:
> > 1. add __no_const to the structure declaration
> > 2. typedef a __no_const version of the constified structure type
> > 3. use pax_open_kernel/pax_close_kernel to temporarily override
> > * the (runtime enforced) constness of a given variable
> > each approach has its own conditions to use, here's a quick summary:
> > 1. when all instances of the given structure type are runtime allocated
> > * (i.e., there're no static instances)
> > 2. when some instances of the given structure type are statically allocated
> > * it's best to let them be consitified by the plugin and use the typedef'd
> > * __no_const type for dynamically allocated ones
> > 3. when some constified statically allocated variables do need to be modified
> > * at runtime
> > if you look at PaX carefully, you'll find use cases for each of the above,
> > it should be your guidance for patching aufs as well. although i didn't look
> > at its code, i think aufs is case #1 or #2.
> I agree with option 1: types should be declared const or no_const
> As enforcing this should lead to a more robust kernel.
> And this means there should not be any constify_plugin trick to infer
> if a type is const or not.
> The compiler should require attribute declaration.
> But this is just utopia....
i think you misunderstood something
. the above 3 cases are not something you
can agree/disagree with, they're cases that may or may not apply to a given piece
of code and depending on the exact situation you have 3 ways to adjust the code
to work with the constification plugin (it is also a valid/possible adjustment
to turn the code from one case to another, e.g., by getting rid of runtime ops
structure allocation, PaX has examples of this kind of change too, but i didn't
want to complicate my initial explanation
so with that said, the constify plugin has to exist exactly because the kernel
has all kinds of ops structure types (as in, there're legitimate use cases for
each of the 3 situations) and we need the do_const/no_const attributes because
the compiler cannot in general determine how a given ops structure type is used
in the entire code base (with LTO it may be feasible though).
> The problem with option 2 is that this leaves the constification
> question unsolved.
i don't understand this...
> Beside that this forces developers to modify their code either to use
> "no_const" types or to write specific patches.
exactly. consider this as documenting the behaviour of the code. not unlike
you do every time you use 'const' or 'unsigned' or any attributes, etc.
> Looking at code I don't understand why, for example, grsecurity patch
> provides a "no_const" struct for seq_operations (in include/linux/seq_file.h)
it's simple, you'd have had to grep for seq_operations_no_const only
(hint: it's case #2).
> and no special "typedef" for "struct fsnotify_ops"...
because fsnotify_ops is not case #2 in the vanilla kernel but now you appear
to be saying that aufs makes it into case #2 (can you/anyone confirm it?) in
which case i'll add the typedef to PaX and aufs can rely on fsnotify_ops_no_const
from then on.
and if there's anything else, just let me know (obviously for ops structures
declared by aufs itself it's up to aufs to provide/use the typedefs, __no_const,
> This means I cannot support option 1 and then the best is to bypass
> compiler constification, using Okajima trick...
> //br->br_hfsn_ops = au_hfsn_ops;
> BUILD_BUG_ON(sizeof(br->br_hfsn_ops) != sizeof(au_hfsn_ops));
> memcoy((void *)&br->br_hfsn_ops, (void *)&au_hfsn_ops, sizeof(au_hfsn_ops));
> instead of the more clear:
> br->br_hfsn_ops = au_hfsn_ops;
you can keep using the simple code if you use the proper types
> Is this going to work?
you/someone will have to determine which of the 3 cases you have in aufs (for
each affected type).
here, if 'br' points to a dynamically allocated structure (think kmalloc) then
it means that for this ops type you have case #1 or #2, so you'll either need
__no_const on the original declaration or a new typedef'ed no_const type.
> Aren't const struct enforced at runtime?
it depends on where the given object is stored. runtime enforcement normally
applies only to variables of static storage (i.e., variables declared in global
scope), local variables and dynamically allocated ones remain writable (as far
as the CPU is concerned, that's why cast tricks work there whereas they'd fail
at runtime if used against static variables). with some ugly hacks you could make
dynamically allocated objects read-only as well, but i don't know of anyone who
went that far yet.