Re: [PATCH v15 11/23] LSM: Use lsmblob in security_cred_getsecid

Paul Moore
2020-03-07T01:36:33Z

On Fri, Feb 21, 2020 at 7:05 PM Casey Schaufler <casey@schaufler-ca.com> wrote:
>
> Change the security_cred_getsecid() interface to fill in a
> lsmblob instead of a u32 secid. The associated data elements
> in the audit sub-system are changed from a secid to a lsmblob
> to accommodate multiple possible LSM audit users.
>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: John Johansen <john.johansen@canonical.com>
> Acked-by: Stephen Smalley <sds@tycho.nsa.gov>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> cc: linux-integrity@vger.kernel.org
> ---
> include/linux/security.h | 2 +-
> kernel/audit.c | 19 +++++++-----------
> kernel/audit.h | 5 +++--
> kernel/auditsc.c | 33 +++++++++++--------------------
> security/integrity/ima/ima_main.c | 8 ++++----
> security/security.c | 12 ++++++++---
> 6 files changed, 36 insertions(+), 43 deletions(-)

There is some undefined scaffolding in the IMA section, but I'll leave
that to Mimi if she cares or not. One small suggestion below, but I'm
okay if you ignore that, it's pretty minor.

Acked-by: Paul Moore <paul@paul-moore.com>

> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index b55e66c2451d..d52ae228ad3d 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1733,7 +1732,7 @@ void __audit_syscall_exit(int success, long return_code)
> context->aux = NULL;
> context->aux_pids = NULL;
> context->target_pid = 0;
> - context->target_sid = 0;
> + lsmblob_init(&context->target_lsm, 0);

Would it be worth having a "lsmblob_unset(struct lsmblob *)" for
situations such as these? Even if right now it is just a wrapper
around "lsmblob_init(blob, 0)" I think it might have some
futureproofing value in case the struct grows additional fields and is
no longer tightly packed.

> context->sockaddr_len = 0;
> context->type = 0;
> context->fds[0] = -1;

--
paul moore