Re: [PATCH v5] evm: Correct inode_init_security hooks behaviors

[Nicolas Bouchinet <nicolas.bouchinet@clip-os.org>]

Hi Mimi,

On Tue, Nov 29, 2022 at 06:28:09AM -0500, Mimi Zohar wrote:
> On Fri, 2022-11-25 at 16:57 +0100, Nicolas Bouchinet wrote:
> > From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> > 
> > Fixes a NULL pointer dereference occurring in the
> > `evm_protected_xattr_common` function of the EVM LSM. The bug is
> > triggered if a `inode_init_security` hook returns 0 without initializing
> > the given `struct xattr` fields (which is the case of BPF) and if no
> > other LSM overrides thoses fields after. This also leads to memory
> > leaks.
> > 
> > The `call_int_hook_xattr` macro has been inlined into the
> > `security_inode_init_security` hook in order to check hooks return
> > values and skip ones who doesn't init `xattrs`.
> > 
> > Modify `evm_init_hmac` function to init the EVM hmac using every
> > entry of the given xattr array.
> > 
> > The `MAX_LSM_EVM_XATTR` value is now based on the security modules
> > compiled in, which gives room for SMACK, SELinux, Apparmor, BPF and
> > IMA/EVM security attributes.
> > 
> > Changes the default return value of the `inode_init_security` hook
> > definition to `-EOPNOTSUPP`.
> > 
> > Changes the hook documentation to match the behavior of the LSMs using
> > it (only xattr->value is initialised with kmalloc and thus is the only
> > one that should be kfreed by the caller).
> > 
> > Cc: roberto.sassu@huaweicloud.com
> > Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr>
> 
> What  is the relationship between this patch and Roberto's patch set? 
> Roberto, if there is an overlap, then at minimum there should be a
> Reported-by tag indicating that your patch set addresses a bug reported
> by Nicolas.

This patch fixes the EVM NULL pointer dereference I have reported, and additionally
improves the stackability of this LSM hook. This latter improvement was originally
addressed by Roberto's patchset, and thus I see no problem for my fix to be merged
within his patchset.
> 
> -- 
> thanks,
> 
> Mimi
> 

Thanks for your time,

Nicolas Bouchinet