[PATCH] sane security_getprocattr() API

[Stephen Smalley]

On Wed, 2007-02-21 at 14:12 -0500, Alexander Viro wrote:
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 3599558..89875b2 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -741,26 +741,18 @@ void audit_log_task_context(struct audit_buffer *ab)
>  	char *ctx = NULL;
>  	ssize_t len = 0;
>  
> -	len = security_getprocattr(current, "current", NULL, 0);
> -	if (len < 0) {
> +	len = security_getprocattr(current, "current", &ctx);
> +	if (len <= 0) {
>  		if (len != -EINVAL)
>  			goto error_path;
>  		return;
>  	}
>  
> -	ctx = kmalloc(len, GFP_KERNEL);
> -	if (!ctx)
> -		goto error_path;
> -
> -	len = security_getprocattr(current, "current", ctx, len);
> -	if (len < 0 )
> -		goto error_path;
> -
>  	audit_log_format(ab, " subj=%s", ctx);
> +	kfree(ctx);
>  	return;
>  
>  error_path:
> -	kfree(ctx);
>  	audit_panic("error in audit_log_task_context");
>  	return;
>  }

The security_getprocattr API change makes sense independently of audit,
but for audit, as I mentioned earlier, I think it would make more sense
to use selinux_get_task_sid() and selinux_sid_to_string() instead of
security_getprocattr(), particularly as audit already calls those
functions for other purposes.  Or if concerned about generality, audit
could call security_task_getsecid() and security_secid_to_secctx().
Also avoids issues with other logic in security_getprocattr that don't
make sense when used by audit, like permission checking logic (although
in the current == p case, it doesn't matter) or support for attributes
other than "current".

-- 
Stephen Smalley
National Security Agency