[PATCH v4 00/19] LSM: Module stacking for SARA and Landlock

On 2018/09/25 1:15, Casey Schaufler wrote:

quoted

quoted

quoted

?? Since all free hooks are called when one of init hooks failed, each
?? free hook needs to check whether init hook was called. An example is
?? inode_free_security() in security/selinux/hooks.c (but not addressed in
?? this patch).

I *think* that selinux_inode_free_security() is safe in this
case because the blob will be zeroed, hence isec->list will
be NULL.

That's not safe - look more closely at what list_empty_careful() tests, and then think about what happens when list_del_init() gets called on that isec->list.? selinux_inode_free_security() presumes that selinux_inode_alloc_security() has been called already.? If you are breaking that assumption, you have to fix it.

Yup. I misread the macro my first time around. Easy fix.

Oh, I didn't notice that it is doing !list_empty_careful() than list_empty_careful().
Unsafe indeed. But easy to fix.

quoted

Is there a reason you can't make inode_alloc_security() return void since you moved the allocation to the framework? 

No reason with any of the existing modules, But I could see someone
doing unnatural things during allocation that might result in a
failure.

Currently upstreamed LSM modules and AKARI would be OK. But I can't guarantee it
for future / not-yet-upstreamed LSM modules.