lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20190125183336.d4xlpdefz7uazqac@madcap2.tricolour.ca>
Date:   Fri, 25 Jan 2019 13:33:36 -0500
From:   Richard Guy Briggs <rgb@...hat.com>
To:     LKML <linux-kernel@...r.kernel.org>,
        Linux-Audit Mailing List <linux-audit@...hat.com>
Subject: Re: [PATCH ghak105 V1 2/2] audit: remove audit_context when CONFIG_
 AUDIT and not AUDITSYSCALL

On 2019-01-22 17:07, Richard Guy Briggs wrote:
> Remove audit_context from struct task_struct and struct audit_buffer
> when CONFIG_AUDIT is enabled but CONFIG_AUDITSYSCALL is not.
> 
> Also, audit_log_name() (and supporting inode and fcaps functions) should
> have been put back in auditsc.c when soft and hard link logging was
> normalized since it is only used by syscall auditing.
> 
> See github issue https://github.com/linux-audit/audit-kernel/issues/105
> 
> Signed-off-by: Richard Guy Briggs <rgb@...hat.com>
> ---
>  include/linux/sched.h |   2 +-
>  kernel/audit.c        | 155 +++-----------------------------------------------
>  kernel/audit.h        |   9 ---
>  kernel/auditsc.c      | 148 +++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 157 insertions(+), 157 deletions(-)
> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index f9788bb122c5..a3a5c657cae9 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -885,8 +885,8 @@ struct task_struct {
>  
>  	struct callback_head		*task_works;
>  
> -	struct audit_context		*audit_context;
>  #ifdef CONFIG_AUDIT
> +	struct audit_context		*audit_context;

This isn't quite right...  audit_context should be wrapped with
CONFIG_AUDITSYSCALL.

>  	kuid_t				loginuid;
>  	unsigned int			sessionid;
>  #endif
> diff --git a/kernel/audit.c b/kernel/audit.c
> index dc375857c59b..79bc49e5162a 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -205,7 +205,9 @@ struct audit_net {
>   * use simultaneously. */
>  struct audit_buffer {
>  	struct sk_buff       *skb;	/* formatted skb ready to send */
> +#ifdef CONFIG_AUDITSYSCALL
>  	struct audit_context *ctx;	/* NULL or associated context */
> +#endif
>  	gfp_t		     gfp_mask;
>  };
>  
> @@ -1695,7 +1697,9 @@ static struct audit_buffer *audit_buffer_alloc(struct audit_context *ctx,
>  	if (!nlmsg_put(ab->skb, 0, 0, type, 0, 0))
>  		goto err;
>  
> +#ifdef CONFIG_AUDITSYSCALL
>  	ab->ctx = ctx;
> +#endif
>  	ab->gfp_mask = gfp_mask;
>  
>  	return ab;
> @@ -1808,7 +1812,11 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
>  		return NULL;
>  	}
>  
> +#ifdef CONFIG_AUDITSYSCALL
>  	audit_get_stamp(ab->ctx, &t, &serial);
> +#else
> +	audit_get_stamp(NULL, &t, &serial);
> +#endif
>  	audit_log_format(ab, "audit(%llu.%03lu:%u): ",
>  			 (unsigned long long)t.tv_sec, t.tv_nsec/1000000, serial);
>  
> @@ -2066,153 +2074,6 @@ void audit_log_key(struct audit_buffer *ab, char *key)
>  		audit_log_format(ab, "(null)");
>  }
>  
> -void audit_log_cap(struct audit_buffer *ab, char *prefix, kernel_cap_t *cap)
> -{
> -	int i;
> -
> -	if (cap_isclear(*cap)) {
> -		audit_log_format(ab, " %s=0", prefix);
> -		return;
> -	}
> -	audit_log_format(ab, " %s=", prefix);
> -	CAP_FOR_EACH_U32(i)
> -		audit_log_format(ab, "%08x", cap->cap[CAP_LAST_U32 - i]);
> -}
> -
> -static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name)
> -{
> -	audit_log_cap(ab, "cap_fp", &name->fcap.permitted);
> -	audit_log_cap(ab, "cap_fi", &name->fcap.inheritable);
> -	audit_log_format(ab, " cap_fe=%d cap_fver=%x",
> -			 name->fcap.fE, name->fcap_ver);
> -}
> -
> -static inline int audit_copy_fcaps(struct audit_names *name,
> -				   const struct dentry *dentry)
> -{
> -	struct cpu_vfs_cap_data caps;
> -	int rc;
> -
> -	if (!dentry)
> -		return 0;
> -
> -	rc = get_vfs_caps_from_disk(dentry, &caps);
> -	if (rc)
> -		return rc;
> -
> -	name->fcap.permitted = caps.permitted;
> -	name->fcap.inheritable = caps.inheritable;
> -	name->fcap.fE = !!(caps.magic_etc & VFS_CAP_FLAGS_EFFECTIVE);
> -	name->fcap_ver = (caps.magic_etc & VFS_CAP_REVISION_MASK) >>
> -				VFS_CAP_REVISION_SHIFT;
> -
> -	return 0;
> -}
> -
> -/* Copy inode data into an audit_names. */
> -void audit_copy_inode(struct audit_names *name, const struct dentry *dentry,
> -		      struct inode *inode)
> -{
> -	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, &name->osid);
> -	audit_copy_fcaps(name, dentry);
> -}
> -
> -/**
> - * audit_log_name - produce AUDIT_PATH record from struct audit_names
> - * @context: audit_context for the task
> - * @n: audit_names structure with reportable details
> - * @path: optional path to report instead of audit_names->name
> - * @record_num: record number to report when handling a list of names
> - * @call_panic: optional pointer to int that will be updated if secid fails
> - */
> -void audit_log_name(struct audit_context *context, struct audit_names *n,
> -		    const struct path *path, int record_num, int *call_panic)
> -{
> -	struct audit_buffer *ab;
> -	ab = audit_log_start(context, GFP_KERNEL, AUDIT_PATH);
> -	if (!ab)
> -		return;
> -
> -	audit_log_format(ab, "item=%d", record_num);
> -
> -	if (path)
> -		audit_log_d_path(ab, " name=", path);
> -	else if (n->name) {
> -		switch (n->name_len) {
> -		case AUDIT_NAME_FULL:
> -			/* log the full path */
> -			audit_log_format(ab, " name=");
> -			audit_log_untrustedstring(ab, n->name->name);
> -			break;
> -		case 0:
> -			/* name was specified as a relative path and the
> -			 * directory component is the cwd */
> -			audit_log_d_path(ab, " name=", &context->pwd);
> -			break;
> -		default:
> -			/* log the name's directory component */
> -			audit_log_format(ab, " name=");
> -			audit_log_n_untrustedstring(ab, n->name->name,
> -						    n->name_len);
> -		}
> -	} else
> -		audit_log_format(ab, " name=(null)");
> -
> -	if (n->ino != AUDIT_INO_UNSET)
> -		audit_log_format(ab, " inode=%lu"
> -				 " dev=%02x:%02x mode=%#ho"
> -				 " ouid=%u ogid=%u rdev=%02x:%02x",
> -				 n->ino,
> -				 MAJOR(n->dev),
> -				 MINOR(n->dev),
> -				 n->mode,
> -				 from_kuid(&init_user_ns, n->uid),
> -				 from_kgid(&init_user_ns, n->gid),
> -				 MAJOR(n->rdev),
> -				 MINOR(n->rdev));
> -	if (n->osid != 0) {
> -		char *ctx = NULL;
> -		u32 len;
> -		if (security_secid_to_secctx(
> -			n->osid, &ctx, &len)) {
> -			audit_log_format(ab, " osid=%u", n->osid);
> -			if (call_panic)
> -				*call_panic = 2;
> -		} else {
> -			audit_log_format(ab, " obj=%s", ctx);
> -			security_release_secctx(ctx, len);
> -		}
> -	}
> -
> -	/* log the audit_names record type */
> -	switch(n->type) {
> -	case AUDIT_TYPE_NORMAL:
> -		audit_log_format(ab, " nametype=NORMAL");
> -		break;
> -	case AUDIT_TYPE_PARENT:
> -		audit_log_format(ab, " nametype=PARENT");
> -		break;
> -	case AUDIT_TYPE_CHILD_DELETE:
> -		audit_log_format(ab, " nametype=DELETE");
> -		break;
> -	case AUDIT_TYPE_CHILD_CREATE:
> -		audit_log_format(ab, " nametype=CREATE");
> -		break;
> -	default:
> -		audit_log_format(ab, " nametype=UNKNOWN");
> -		break;
> -	}
> -
> -	audit_log_fcaps(ab, n);
> -	audit_log_end(ab);
> -}
> -
>  int audit_log_task_context(struct audit_buffer *ab)
>  {
>  	char *ctx = NULL;
> diff --git a/kernel/audit.h b/kernel/audit.h
> index 22ef49b76daa..13210cc52100 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -212,15 +212,6 @@ struct audit_context {
>  
>  extern void audit_log_session_info(struct audit_buffer *ab);
>  
> -extern void audit_copy_inode(struct audit_names *name,
> -			     const struct dentry *dentry,
> -			     struct inode *inode);
> -extern void audit_log_cap(struct audit_buffer *ab, char *prefix,
> -			  kernel_cap_t *cap);
> -extern void audit_log_name(struct audit_context *context,
> -			   struct audit_names *n, const struct path *path,
> -			   int record_num, int *call_panic);
> -
>  extern int auditd_test_task(struct task_struct *task);
>  
>  #define AUDIT_INODE_BUCKETS	32
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 572d247957fb..e8f257fbddaf 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1136,6 +1136,27 @@ static void audit_log_execve_info(struct audit_context *context,
>  	kfree(buf_head);
>  }
>  
> +void audit_log_cap(struct audit_buffer *ab, char *prefix, kernel_cap_t *cap)
> +{
> +	int i;
> +
> +	if (cap_isclear(*cap)) {
> +		audit_log_format(ab, " %s=0", prefix);
> +		return;
> +	}
> +	audit_log_format(ab, " %s=", prefix);
> +	CAP_FOR_EACH_U32(i)
> +		audit_log_format(ab, "%08x", cap->cap[CAP_LAST_U32 - i]);
> +}
> +
> +static void audit_log_fcaps(struct audit_buffer *ab, struct audit_names *name)
> +{
> +	audit_log_cap(ab, "cap_fp", &name->fcap.permitted);
> +	audit_log_cap(ab, "cap_fi", &name->fcap.inheritable);
> +	audit_log_format(ab, " cap_fe=%d cap_fver=%x",
> +			 name->fcap.fE, name->fcap_ver);
> +}
> +
>  static void show_special(struct audit_context *context, int *call_panic)
>  {
>  	struct audit_buffer *ab;
> @@ -1258,6 +1279,97 @@ static inline int audit_proctitle_rtrim(char *proctitle, int len)
>  	return len;
>  }
>  
> +/*
> + * audit_log_name - produce AUDIT_PATH record from struct audit_names
> + * @context: audit_context for the task
> + * @n: audit_names structure with reportable details
> + * @path: optional path to report instead of audit_names->name
> + * @record_num: record number to report when handling a list of names
> + * @call_panic: optional pointer to int that will be updated if secid fails
> + */
> +static void audit_log_name(struct audit_context *context, struct audit_names *n,
> +		    const struct path *path, int record_num, int *call_panic)
> +{
> +	struct audit_buffer *ab;
> +
> +	ab = audit_log_start(context, GFP_KERNEL, AUDIT_PATH);
> +	if (!ab)
> +		return;
> +
> +	audit_log_format(ab, "item=%d", record_num);
> +
> +	if (path)
> +		audit_log_d_path(ab, " name=", path);
> +	else if (n->name) {
> +		switch (n->name_len) {
> +		case AUDIT_NAME_FULL:
> +			/* log the full path */
> +			audit_log_format(ab, " name=");
> +			audit_log_untrustedstring(ab, n->name->name);
> +			break;
> +		case 0:
> +			/* name was specified as a relative path and the
> +			 * directory component is the cwd
> +			 */
> +			audit_log_d_path(ab, " name=", &context->pwd);
> +			break;
> +		default:
> +			/* log the name's directory component */
> +			audit_log_format(ab, " name=");
> +			audit_log_n_untrustedstring(ab, n->name->name,
> +						    n->name_len);
> +		}
> +	} else
> +		audit_log_format(ab, " name=(null)");
> +
> +	if (n->ino != AUDIT_INO_UNSET)
> +		audit_log_format(ab, " inode=%lu dev=%02x:%02x mode=%#ho ouid=%u ogid=%u rdev=%02x:%02x",
> +				 n->ino,
> +				 MAJOR(n->dev),
> +				 MINOR(n->dev),
> +				 n->mode,
> +				 from_kuid(&init_user_ns, n->uid),
> +				 from_kgid(&init_user_ns, n->gid),
> +				 MAJOR(n->rdev),
> +				 MINOR(n->rdev));
> +	if (n->osid != 0) {
> +		char *ctx = NULL;
> +		u32 len;
> +
> +		if (security_secid_to_secctx(
> +			n->osid, &ctx, &len)) {
> +			audit_log_format(ab, " osid=%u", n->osid);
> +			if (call_panic)
> +				*call_panic = 2;
> +		} else {
> +			audit_log_format(ab, " obj=%s", ctx);
> +			security_release_secctx(ctx, len);
> +		}
> +	}
> +
> +	/* log the audit_names record type */
> +	switch (n->type) {
> +	case AUDIT_TYPE_NORMAL:
> +		audit_log_format(ab, " nametype=NORMAL");
> +		break;
> +	case AUDIT_TYPE_PARENT:
> +		audit_log_format(ab, " nametype=PARENT");
> +		break;
> +	case AUDIT_TYPE_CHILD_DELETE:
> +		audit_log_format(ab, " nametype=DELETE");
> +		break;
> +	case AUDIT_TYPE_CHILD_CREATE:
> +		audit_log_format(ab, " nametype=CREATE");
> +		break;
> +	default:
> +		audit_log_format(ab, " nametype=UNKNOWN");
> +		break;
> +	}
> +
> +	audit_log_fcaps(ab, n);
> +	audit_log_end(ab);
> +}
> +
>  static void audit_log_proctitle(void)
>  {
>  	int res;
> @@ -1750,6 +1862,42 @@ void __audit_getname(struct filename *name)
>  		get_fs_pwd(current->fs, &context->pwd);
>  }
>  
> +static inline int audit_copy_fcaps(struct audit_names *name,
> +				   const struct dentry *dentry)
> +{
> +	struct cpu_vfs_cap_data caps;
> +	int rc;
> +
> +	if (!dentry)
> +		return 0;
> +
> +	rc = get_vfs_caps_from_disk(dentry, &caps);
> +	if (rc)
> +		return rc;
> +
> +	name->fcap.permitted = caps.permitted;
> +	name->fcap.inheritable = caps.inheritable;
> +	name->fcap.fE = !!(caps.magic_etc & VFS_CAP_FLAGS_EFFECTIVE);
> +	name->fcap_ver = (caps.magic_etc & VFS_CAP_REVISION_MASK) >>
> +				VFS_CAP_REVISION_SHIFT;
> +
> +	return 0;
> +}
> +
> +/* Copy inode data into an audit_names. */
> +void audit_copy_inode(struct audit_names *name, const struct dentry *dentry,
> +		      struct inode *inode)
> +{
> +	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, &name->osid);
> +	audit_copy_fcaps(name, dentry);
> +}
> +
>  /**
>   * __audit_inode - store the inode and device from a lookup
>   * @name: name being audited
> -- 
> 1.8.3.1
> 
> --
> Linux-audit mailing list
> Linux-audit@...hat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb@...hat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ