Re: [PATCH v35 22/29] Audit: Keep multiple LSM data in audit_names

[John Johansen]

On 4/18/22 07:59, Casey Schaufler wrote:
> Replace the osid field in the audit_names structure
> with a lsmblob structure. This accomodates the use
> of an lsmblob in security_audit_rule_match() and
> security_inode_getsecid().
> 
> Signed-off-by: Casey Schaufler <casey@xxxxxxxxxxxxxxxx>
> Acked-by: Paul Moore <paul@xxxxxxxxxxxxxx>
> ---
>  kernel/audit.h   |  2 +-
>  kernel/auditsc.c | 22 ++++++++--------------
>  2 files changed, 9 insertions(+), 15 deletions(-)
> 
> diff --git a/kernel/audit.h b/kernel/audit.h
> index 316fac62d5f7..4af63e7dde17 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -82,7 +82,7 @@ struct audit_names {
>  	kuid_t			uid;
>  	kgid_t			gid;
>  	dev_t			rdev;
> -	u32			osid;
> +	struct lsmblob		lsmblob;
>  	struct audit_cap_data	fcap;
>  	unsigned int		fcap_ver;
>  	unsigned char		type;		/* record type */
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 231631f61550..6fe9f2525fc1 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -700,17 +700,16 @@ static int audit_filter_rules(struct task_struct *tsk,
>  					 * lsmblob, which happens later in
>  					 * this patch set.
>  					 */
> -					lsmblob_init(&blob, name->osid);
>  					result = security_audit_rule_match(
> -								&blob,
> +								&name->lsmblob,
>  								f->type,
>  								f->op,
>  								&f->lsm_rules);
>  				} else if (ctx) {
>  					list_for_each_entry(n, &ctx->names_list, list) {
> -						lsmblob_init(&blob, n->osid);
>  						if (security_audit_rule_match(
> -							&blob, f->type, f->op,
> +							&n->lsmblob,
> +							f->type, f->op,
>  							&f->lsm_rules)) {
>  							++result;
>  							break;
> @@ -1589,13 +1588,12 @@ static void audit_log_name(struct audit_context *context, struct audit_names *n,
>  				 from_kgid(&init_user_ns, n->gid),
>  				 MAJOR(n->rdev),
>  				 MINOR(n->rdev));
> -	if (n->osid != 0) {
> -		struct lsmblob blob;
> +	if (lsmblob_is_set(&n->lsmblob)) {
>  		struct lsmcontext lsmctx;
>  
> -		lsmblob_init(&blob, n->osid);
> -		if (security_secid_to_secctx(&blob, &lsmctx, LSMBLOB_FIRST)) {
> -			audit_log_format(ab, " osid=%u", n->osid);
> +		if (security_secid_to_secctx(&n->lsmblob, &lsmctx,
> +					     LSMBLOB_FIRST)) {
> +			audit_log_format(ab, " osid=?");

is there something better we can do here? This feels like a regression

>  			if (call_panic)
>  				*call_panic = 2;
>  		} else {
> @@ -2297,17 +2295,13 @@ static void audit_copy_inode(struct audit_names *name,
>  			     const struct dentry *dentry,
>  			     struct inode *inode, unsigned int flags)
>  {
> -	struct lsmblob blob;
> -
>  	name->ino   = inode->i_ino;
>  	name->dev   = inode->i_sb->s_dev;
>  	name->mode  = inode->i_mode;
>  	name->uid   = inode->i_uid;
>  	name->gid   = inode->i_gid;
>  	name->rdev  = inode->i_rdev;
> -	security_inode_getsecid(inode, &blob);
> -	/* scaffolding until osid is updated */
> -	name->osid = lsmblob_first(&blob);
> +	security_inode_getsecid(inode, &name->lsmblob);
>  	if (flags & AUDIT_INODE_NOEVAL) {
>  		name->fcap_ver = -1;
>  		return;