[PATCH 2/2] filesystem auditing: augment audit_inode
Timothy R. Chavez
tinytim at us.ibm.com
Thu Oct 20 15:04:09 UTC 2005
Not too many comments on my first cursory glance. I'll go ahead and incorporate this patch into my patch set.
-tim
On Wednesday 19 October 2005 16:11, Amy Griffis wrote:
> Collect more inode information during syscall processing.
>
> NOTE: This patch makes some changes to the output of AUDIT_PATH
> records. In the case of the name field, the record will show
> "name=(null)" if there is no name field (e.g. in an fchown call). I
> did this because it seemed it would make more sense to someone looking
> at the records.
>
> I also added a "parent" field to distinguish between the inode number
> and the parent inode number. This allowed me to remove the "flags"
> field. In some cases, such as syscall failures, inode information may
> not be present in the audit context. I took the liberty to not emit
> fields with undefined values. I don't know if this is the right
> approach. I think the real solution is to move to a binary record
> format and leave this decision for a userspace tool.
I definately agree with you with regards to the binary record.
>
>
> diff --git a/fs/namei.c b/fs/namei.c
> --- a/fs/namei.c
> +++ b/fs/namei.c
> @@ -1046,8 +1046,7 @@ int fastcall path_lookup(const char *nam
> current->total_link_count = 0;
> retval = link_path_walk(name, nd);
> out:
> - if (unlikely(current->audit_context
> - && nd && nd->dentry && nd->dentry->d_inode))
> + if (nd && nd->dentry && nd->dentry->d_inode)
> audit_inode(name, nd->dentry->d_inode, flags);
> return retval;
> }
> @@ -1192,6 +1191,7 @@ static inline int may_delete(struct inod
> return -ENOENT;
>
> BUG_ON(victim->d_parent->d_inode != dir);
> + audit_inode_child(victim->d_name.name, victim->d_inode, dir->i_ino);
>
> error = permission(dir,MAY_WRITE | MAY_EXEC, NULL);
> if (error)
> diff --git a/fs/open.c b/fs/open.c
> --- a/fs/open.c
> +++ b/fs/open.c
> @@ -25,6 +25,7 @@
> #include <linux/pagemap.h>
> #include <linux/syscalls.h>
> #include <linux/rcupdate.h>
> +#include <linux/audit.h>
>
> #include <asm/unistd.h>
>
> @@ -609,6 +610,8 @@ asmlinkage long sys_fchmod(unsigned int
> dentry = file->f_dentry;
> inode = dentry->d_inode;
>
> + audit_inode(NULL, inode, 0);
> +
> err = -EROFS;
> if (IS_RDONLY(inode))
> goto out_putf;
> @@ -732,7 +735,10 @@ asmlinkage long sys_fchown(unsigned int
>
> file = fget(fd);
> if (file) {
> - error = chown_common(file->f_dentry, user, group);
> + struct dentry * dentry;
I guess for consistency's sake this should be declared at the top?
> + dentry = file->f_dentry;
> + audit_inode(NULL, dentry->d_inode, 0);
> + error = chown_common(dentry, user, group);
> fput(file);
> }
> return error;
> diff --git a/fs/xattr.c b/fs/xattr.c
> --- a/fs/xattr.c
> +++ b/fs/xattr.c
> @@ -17,6 +17,7 @@
> #include <linux/syscalls.h>
> #include <linux/module.h>
> #include <linux/fsnotify.h>
> +#include <linux/audit.h>
> #include <asm/uaccess.h>
>
> /*
> @@ -114,12 +115,15 @@ sys_fsetxattr(int fd, char __user *name,
> size_t size, int flags)
> {
> struct file *f;
> + struct dentry *dentry;
> int error = -EBADF;
>
> f = fget(fd);
> if (!f)
> return error;
> - error = setxattr(f->f_dentry, name, value, size, flags);
> + dentry = f->f_dentry;
> + audit_inode(NULL, dentry->d_inode, 0);
> + error = setxattr(dentry, name, value, size, flags);
> fput(f);
> return error;
> }
> @@ -364,12 +368,15 @@ asmlinkage long
> sys_fremovexattr(int fd, char __user *name)
> {
> struct file *f;
> + struct dentry *dentry;
> int error = -EBADF;
>
> f = fget(fd);
> if (!f)
> return error;
> - error = removexattr(f->f_dentry, name);
> + dentry = f->f_dentry;
> + audit_inode(NULL, dentry->d_inode, 0);
> + error = removexattr(dentry, name);
> fput(f);
> return error;
> }
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -222,7 +222,20 @@ extern void audit_syscall_entry(struct t
> extern void audit_syscall_exit(struct task_struct *task, int failed, long return_code);
> extern void audit_getname(const char *name);
> extern void audit_putname(const char *name);
> -extern void audit_inode(const char *name, const struct inode *inode, unsigned flags);
> +extern void __audit_inode(const char *name, const struct inode *inode, unsigned flags);
> +extern void __audit_inode_child(const char *dname, const struct inode *inode,
> + unsigned long pino);
> +static inline void audit_inode(const char *name, const struct inode *inode,
> + unsigned flags) {
> + if (unlikely(current->audit_context))
> + __audit_inode(name, inode, flags);
> +}
> +static inline void audit_inode_child(const char *dname,
> + const struct inode *inode,
> + unsigned long pino) {
> + if (unlikely(current->audit_context))
> + __audit_inode_child(dname, inode, pino);
> +}
>
> /* Private API (for audit.c only) */
> extern int audit_receive_filter(int type, int pid, int uid, int seq,
> @@ -245,7 +258,10 @@ extern int audit_filter_user(struct netl
> #define audit_syscall_exit(t,f,r) do { ; } while (0)
> #define audit_getname(n) do { ; } while (0)
> #define audit_putname(n) do { ; } while (0)
> +#define __audit_inode(n,i,f) do { ; } while (0)
> +#define __audit_inode_child(d,i,p) do { ; } while (0)
> #define audit_inode(n,i,f) do { ; } while (0)
> +#define audit_inode_child(d,i,p) do { ; } while (0)
> #define audit_receive_filter(t,p,u,s,d,l) ({ -EOPNOTSUPP; })
> #define auditsc_get_stamp(c,t,s) do { BUG(); } while (0)
> #define audit_get_loginuid(c) ({ -1; })
> diff --git a/include/linux/fsnotify.h b/include/linux/fsnotify.h
> --- a/include/linux/fsnotify.h
> +++ b/include/linux/fsnotify.h
> @@ -15,6 +15,7 @@
>
> #include <linux/dnotify.h>
> #include <linux/inotify.h>
> +#include <linux/audit.h>
>
> /*
> * fsnotify_move - file old_name at old_dir was moved to new_name at new_dir
> @@ -45,6 +46,8 @@ static inline void fsnotify_move(struct
> if (source) {
> inotify_inode_queue_event(source, IN_MOVE_SELF, 0, NULL);
> }
> + audit_inode_child(old_name, source, old_dir->i_ino);
> + audit_inode_child(new_name, target, new_dir->i_ino);
> }
>
> /*
> @@ -74,6 +77,7 @@ static inline void fsnotify_create(struc
> {
> inode_dir_notify(inode, DN_CREATE);
> inotify_inode_queue_event(inode, IN_CREATE, 0, dentry->d_name.name);
> + audit_inode_child(dentry->d_name.name, dentry->d_inode, inode->i_ino);
> }
>
> /*
> @@ -84,6 +88,7 @@ static inline void fsnotify_mkdir(struct
> inode_dir_notify(inode, DN_CREATE);
> inotify_inode_queue_event(inode, IN_CREATE | IN_ISDIR, 0,
> dentry->d_name.name);
> + audit_inode_child(dentry->d_name.name, dentry->d_inode, inode->i_ino);
> }
>
> /*
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -2,6 +2,7 @@
> * Handles all system-call specific auditing features.
> *
> * Copyright 2003-2004 Red Hat Inc., Durham, North Carolina.
> + * Copyright 2005 Hewlett-Packard Development Company, L.P.
> * All Rights Reserved.
> *
> * This program is free software; you can redistribute it and/or modify
> @@ -27,11 +28,15 @@
> * this file -- see entry.S) is based on a GPL'd patch written by
> * okir at suse.de and Copyright 2003 SuSE Linux AG.
> *
> + * Modified by Amy Griffis <amy.griffis at hp.com> to collect additional
> + * filesystem information.
> */
>
> #include <linux/init.h>
> #include <asm/atomic.h>
> #include <asm/types.h>
> +#include <linux/fs.h>
> +#include <linux/namei.h>
> #include <linux/mm.h>
> #include <linux/module.h>
> #include <linux/mount.h>
> @@ -93,12 +98,12 @@ enum audit_state {
> struct audit_names {
> const char *name;
> unsigned long ino;
> + unsigned long pino;
> dev_t dev;
> umode_t mode;
> uid_t uid;
> gid_t gid;
> dev_t rdev;
> - unsigned flags;
> };
>
> struct audit_aux_data {
> @@ -479,7 +484,9 @@ static int audit_filter_rules(struct tas
> case AUDIT_INODE:
> if (ctx) {
> for (j = 0; j < ctx->name_count; j++) {
> - if (ctx->names[j].ino == value) {
> + if ((ctx->names[j].ino == value) ||
> + (ctx->names[j].pino == value))
> + {
> ++result;
> break;
> }
> @@ -663,17 +670,17 @@ static inline void audit_free_names(stru
> #if AUDIT_DEBUG == 2
> if (context->auditable
> ||context->put_count + context->ino_count != context->name_count) {
> - printk(KERN_ERR "audit.c:%d(:%d): major=%d in_syscall=%d"
> + printk(KERN_ERR "%s:%d(:%d): major=%d in_syscall=%d"
> " name_count=%d put_count=%d"
> " ino_count=%d [NOT freeing]\n",
> - __LINE__,
> + __FILE__, __LINE__,
> context->serial, context->major, context->in_syscall,
> context->name_count, context->put_count,
> context->ino_count);
> for (i = 0; i < context->name_count; i++)
> printk(KERN_ERR "names[%d] = %p = %s\n", i,
> context->names[i].name,
> - context->names[i].name);
> + context->names[i].name ?: "(null)");
> dump_stack();
> return;
> }
> @@ -899,27 +906,34 @@ static void audit_log_exit(struct audit_
> }
> }
> for (i = 0; i < context->name_count; i++) {
> + unsigned long ino = context->names[i].ino;
> + unsigned long pino = context->names[i].pino;
> +
> ab = audit_log_start(context, GFP_KERNEL, AUDIT_PATH);
> if (!ab)
> continue; /* audit_panic has been called */
>
> audit_log_format(ab, "item=%d", i);
> - if (context->names[i].name) {
> - audit_log_format(ab, " name=");
> +
> + audit_log_format(ab, " name=");
> + if (context->names[i].name)
> audit_log_untrustedstring(ab, context->names[i].name);
> - }
> - audit_log_format(ab, " flags=%x\n", context->names[i].flags);
> -
> - if (context->names[i].ino != (unsigned long)-1)
> - audit_log_format(ab, " inode=%lu dev=%02x:%02x mode=%#o"
> - " ouid=%u ogid=%u rdev=%02x:%02x",
> - context->names[i].ino,
> - MAJOR(context->names[i].dev),
> - MINOR(context->names[i].dev),
> - context->names[i].mode,
> - context->names[i].uid,
> - context->names[i].gid,
> - MAJOR(context->names[i].rdev),
> + else
> + audit_log_format(ab, "(null)");
> +
> + if (pino != (unsigned long)-1)
> + audit_log_format(ab, " parent=%lu", pino);
> + if (ino != (unsigned long)-1)
> + audit_log_format(ab, " inode=%lu", ino);
> + if ((pino != (unsigned long)-1) || (ino != (unsigned long)-1))
> + audit_log_format(ab, " dev=%02x:%02x mode=%#o"
> + " ouid=%u ogid=%u rdev=%02x:%02x",
> + MAJOR(context->names[i].dev),
> + MINOR(context->names[i].dev),
> + context->names[i].mode,
> + context->names[i].uid,
> + context->names[i].gid,
> + MAJOR(context->names[i].rdev),
> MINOR(context->names[i].rdev));
> audit_log_end(ab);
> }
> @@ -1146,7 +1160,7 @@ void audit_putname(const char *name)
> for (i = 0; i < context->name_count; i++)
> printk(KERN_ERR "name[%d] = %p = %s\n", i,
> context->names[i].name,
> - context->names[i].name);
> + context->names[i].name ?: "(null)");
> }
> #endif
> __putname(name);
> @@ -1174,9 +1188,10 @@ void audit_putname(const char *name)
> * @inode: inode being audited
> * @flags: lookup flags (as used in path_lookup())
> *
> - * Called from fs/namei.c:path_lookup().
> + * Hooking path_lookup() catches most cases. Syscalls operating on
> + * file descriptors must be separately hooked.
> */
> -void audit_inode(const char *name, const struct inode *inode, unsigned flags)
> +void __audit_inode(const char *name, const struct inode *inode, unsigned flags)
> {
> int idx;
> struct audit_context *context = current->audit_context;
> @@ -1202,13 +1217,93 @@ void audit_inode(const char *name, const
> ++context->ino_count;
> #endif
> }
> - context->names[idx].flags = flags;
> - context->names[idx].ino = inode->i_ino;
> context->names[idx].dev = inode->i_sb->s_dev;
> context->names[idx].mode = inode->i_mode;
> context->names[idx].uid = inode->i_uid;
> context->names[idx].gid = inode->i_gid;
> context->names[idx].rdev = inode->i_rdev;
> + if ((flags & LOOKUP_PARENT) && (strcmp(name, "/") != 0) &&
> + (strcmp(name, ".") != 0)) {
> + context->names[idx].ino = (unsigned long)-1;
> + context->names[idx].pino = inode->i_ino;
> + } else {
> + context->names[idx].ino = inode->i_ino;
> + context->names[idx].pino = (unsigned long)-1;
> + }
> +}
> +
> +/**
> + * audit_inode_child - collect inode info for created/removed objects
> + * @dname: inode's dentry name
> + * @inode: inode being audited
> + * @pino: inode number of dentry parent
> + *
> + * For syscalls that create or remove filesystem objects, audit_inode
> + * can only collect information for the filesystem object's parent.
> + * This call updates the audit context with the child's information.
> + * Syscalls that create a new filesystem object must be hooked after
> + * the object is created. Syscalls that remove a filesystem object
> + * must be hooked prior, in order to capture the target inode during
> + * unsuccessful attempts.
> + */
> +void __audit_inode_child(const char *dname, const struct inode *inode,
> + unsigned long pino)
> +{
> + int idx;
> + struct audit_context *context = current->audit_context;
Does this process even if !audit_enabled?
> +
> + if (!context->in_syscall)
> + return;
> +
> + /* determine matching parent */
> + if (dname)
> + for (idx = 0; idx < context->name_count; idx++)
> + if (context->names[idx].pino == pino) {
> + const char *n;
> + const char *name = context->names[idx].name;
> + int dlen = strlen(dname);
> + int nlen = name ? strlen(name) : 0;
> +
> + if (nlen < dlen)
> + continue;
> +
> + /* disregard trailing slashes */
> + n = name + nlen - 1;
> + while ((*n == '/') && (n > name))
> + n--;
> +
> + /* find last path component */
> + n = n - dlen + 1;
> + if (n < name)
> + continue;
> + else if (n > name) {
> + if (*--n != '/')
> + continue;
> + else
> + n++;
> + }
> +
> + if (strncmp(n, dname, dlen) == 0)
> + goto update_context;
> + }
> +
> + /* catch-all in case match not found */
> + idx = context->name_count++;
> + context->names[idx].name = NULL;
> + context->names[idx].pino = pino;
> +#if AUDIT_DEBUG
> + context->ino_count++;
> +#endif
> +
> +update_context:
> + if (inode) {
> + context->names[idx].ino = inode->i_ino;
> + context->names[idx].dev = inode->i_sb->s_dev;
> + context->names[idx].mode = inode->i_mode;
> + context->names[idx].uid = inode->i_uid;
> + context->names[idx].gid = inode->i_gid;
> + context->names[idx].rdev = inode->i_rdev;
> + }
> }
>
> /**
>
> --
> Linux-audit mailing list
> Linux-audit at redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit
>
>
More information about the Linux-audit
mailing list