Re: [PATCH ghak105 V1 2/2] audit: remove audit_context when CONFIG_ AUDIT and not AUDITSYSCALL

From: Richard Guy Briggs
Date: Fri Jan 25 2019 - 13:33:48 EST

Next message: Kees Cook: "[PATCH] selftests/seccomp: Enhance per-arch ptrace syscall skip tests"
Previous message: Johannes Weiner: "Re: + memcg-do-not-report-racy-no-eligible-oom-tasks.patch added to -mm tree"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]

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@xxxxxxxxxx>
> ---
> 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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb@xxxxxxxxxx>
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

Next message: Kees Cook: "[PATCH] selftests/seccomp: Enhance per-arch ptrace syscall skip tests"
Previous message: Johannes Weiner: "Re: + memcg-do-not-report-racy-no-eligible-oom-tasks.patch added to -mm tree"
Messages sorted by: [ date ] [ thread ] [ subject ] [ author ]