On Thu, Oct 20, 2022 at 9:55 AM Nicolas Bouchinet
<nicolas.bouchinet@xxxxxxxxxxx> wrote:
>
> From: Nicolas Bouchinet <nicolas.bouchinet@xxxxxxxxxxx>
>
> Fixes a NULL pointer dereference occuring 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.
You'll have to forgive me, my connection is poor at the moment and my
time is limited, but why not simply add some additional checking at
the top of evm_inode_init_security()? The LSM hook already memset()'s
the passed lsm_attrs to zero so xattr::{name,value,value_len} should
all be zero/NULL. Can you help me understand why that is not
possible?
Based on my current understanding, I believe this is something that
should be addressed at the IMA/EVM level and not necessairly at the
LSM layer.
> Adds a `call_int_hook_xattr` macro that fetches and feed the
> `new_xattrs` array with every called hook xattr values.
>
> Adds a `evm_init_hmacs` function which init the EVM hmac using every
> entry of the array contrary to `evm_init_hmac`.
>
> Fixes the `evm_inode_init_security` function to use `evm_init_hmacs`.
>
> The `MAX_LSM_EVM_XATTR` value has been raised to 5 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: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Nicolas Bouchinet <nicolas.bouchinet@xxxxxxxxxxx>
> ---
> include/linux/lsm_hook_defs.h | 2 +-
> include/linux/lsm_hooks.h | 4 ++--
> security/integrity/evm/evm.h | 2 ++
> security/integrity/evm/evm_crypto.c | 23 ++++++++++++++++++++++-
> security/integrity/evm/evm_main.c | 11 ++++++-----
> security/security.c | 29 ++++++++++++++++++++++++++---
> 6 files changed, 59 insertions(+), 12 deletions(-)
--
paul-moore.com
