On 12/16/21 08:31, Christian Brauner wrote:
1. namespace securityfs
This patch is thematically standalone and should move to the
beginning of the series.
I would strongly recommend to fold patch 9 and 10 into a single patch
and add a lengthy explanation. You should be able to recycle a lof of
stuff I wrote in earlier reviews.
2. Introduce struct ima_namespace and pass it through to all callers:
- introduce struct ima_namespace
- move all the relevant things into this structure (this also avoids
the "avoid_zero_size" hack).
We could defer the kmalloc() that doesn't work on a zero-sized request.
I would say this is minor.
- define, setup, and expose init_ima_ns
- introduce get_current_ns() and always have it return &init_ima_ns for now
- replace all accesses to global variables to go through &init_ima_ns
- add new infrastructure you'll need later on
Bonus is that you can extend all the functions that later need access
to a specific ima namespace to take a struct ima_namespace * argument
and pass down &init_ima_ns down (retrieved via get_current_ns()). This
will make the actual namespace patch very easy to follow.
3. namespace ima
- add a new entry for struct ima_namespace to struct user_namespace
- add creation helpers, kmem cache etc.
- create files in securityfs per ns
I have tried this now and I am looking at 4 remaining patches that need
to somehow find its way into v8 without causing too many disturbances.
At what point (over how many patches) can I introduce CONFIG_IMA_NS
without anything related to IMA namespacing happening? I need it early
in 'your 3rd part' since it is also used for conditional compilation
(Makefile) and #ifdef's where Makefile content and what the #ifdefs are
doing probably shouldn't be squeezed into a single patch just so it's
all enabled in one patch, but it should probably still remain logically
separated into different patches. Enablement of IMA namespace would be
in the very last patch. But there may be several patches between the
very last one and CONFIG_IMA_NS is introduced...
v7 at least, before the requirement to do late/lazy initialization,
enabled CONFIG_IMA_NS right away and built ever step on top of it, even
if the IMA namespace only became **configurable** in the last patch when
securityfs was enbled and one could set a policy. From that perspective
it would be easier to switch to late initialization in a patch on top of
v7 but .. ok, we cannot do that.
This way at all points in the series we have clearly defined semantics
where ima namespacing is either fully working or fully not working and
the switch is atomic in the patch(es) part of 3.
Atomic over multiple patches? So introducing CONFIG_IMA_NS that doesn't
do anything for several patches is still considered 'atomic' then ?
