[RFC] linux-2.6.10-auditfs-tc1.patch
Chris Wright
chrisw at osdl.org
Fri Jan 21 03:20:27 UTC 2005
* Timothy R. Chavez (chavezt at gmail.com) wrote:
> rudimentary file system auditing capability using the in-kernel audit
> subsystem. From userspace, an administrator passes into the kernel a
> pathname and filterkey pair. The terminating file or directory does
> not have to exist at the time a watch point is created for it, but
> it's parent does.
What is a filterkey?
> TODOs:
>
> * Add RCU locking in the appropriate places to make SMP-safe
Is RCU a requirement, or just some real locking?
> * Switch over to slab caches for struct audit_data, struct audit_file,
> and struct audit_watch
Yes, please.
> * Think about how I can make it so an administrator can say, "Watch
> this file only on the syscall open where uid=0" -- As of right now, if
> filesystem auditing is on, every syscall (being audited) that accesses
> a watched file or directory collects information about it and
> generates a record. Is this sufficient?
Seems intuitively useful to at least be able to watch a file regardless
of who touched it, or what syscall was used. I think you'd especially
want to know if you had some non uid=0 process that wrote to a sensitive
file abusing it's CAP_DAC_OVERRIDE privilege for example.
> * Make it so filesystem auditing can be dynamically enabled/disabled
> from userspace, rather then being "enabled" if configured or
> "disabled" if not configured at boot time.
> * Add/remove unnecessary information about the file or directory being
> collected in the audit context. Input on this?
I missed which parts are unnecessary?
diff -Npru linux-2.6.10/Makefile linux-2.6.10-audit/Makefile
--- linux-2.6.10/Makefile 2004-12-24 15:35:01.000000000 -0600
+++ linux-2.6.10-audit/Makefile 2005-01-19 11:17:19.000000000 -0600
@@ -1,8 +1,8 @@
VERSION = 2
PATCHLEVEL = 6
SUBLEVEL = 10
-EXTRAVERSION =
-NAME=Woozy Numbat
+EXTRAVERSION =-auditfs-tc1
+NAME=
Probably not really necessary.
--- linux-2.6.10/arch/i386/defconfig 2004-12-24 15:35:01.000000000
-0600
+++ linux-2.6.10-audit/arch/i386/defconfig 2005-01-07
15:44:26.000000000 -0600
@@ -23,6 +23,7 @@ CONFIG_POSIX_MQUEUE=y
CONFIG_SYSCTL=y
CONFIG_AUDIT=y
CONFIG_AUDITSYSCALL=y
+CONFIG_AUDITFILESYSTEM=y
Yes here, but default Kconfig is N?
diff -Npru linux-2.6.10/fs/dcache.c linux-2.6.10-audit/fs/dcache.c
--- linux-2.6.10/fs/dcache.c 2004-12-24 15:34:00.000000000 -0600
+++ linux-2.6.10-audit/fs/dcache.c 2005-01-18 14:20:18.000000000 -0600
@@ -32,6 +32,7 @@
#include <linux/seqlock.h>
#include <linux/swap.h>
#include <linux/bootmem.h>
+#include <linux/audit.h>
/* #define DCACHE_DEBUG 1 */
@@ -781,6 +782,7 @@ void d_instantiate(struct dentry *entry,
entry->d_inode = inode;
spin_unlock(&dcache_lock);
security_d_instantiate(entry, inode);
+ audit_watch(entry);
}
/**
@@ -920,6 +922,7 @@ struct dentry *d_splice_alias(struct ino
security_d_instantiate(dentry, inode);
d_rehash(dentry);
}
+ audit_watch(dentry);
} else
d_add(dentry, inode);
return new;
@@ -1266,6 +1269,7 @@ already_unhashed:
spin_unlock(&dentry->d_lock);
write_sequnlock(&rename_lock);
spin_unlock(&dcache_lock);
+ audit_watch(dentry);
Hmm, I don't think this does what you mentioned. Here, a rename gets the
new pathname watchpoint, so you could lose the inode watch with a rename.
}
/**
diff -Npru linux-2.6.10/fs/inode.c linux-2.6.10-audit/fs/inode.c
--- linux-2.6.10/fs/inode.c 2004-12-24 15:35:40.000000000 -0600
+++ linux-2.6.10-audit/fs/inode.c 2005-01-17 17:44:59.000000000 -0600
@@ -135,6 +135,7 @@ static struct inode *alloc_inode(struct
inode->i_cdev = NULL;
inode->i_rdev = 0;
inode->i_security = NULL;
+ inode->i_audit = NULL;
inode->dirtied_when = 0;
if (security_inode_alloc(inode)) {
if (inode->i_sb->s_op->destroy_inode)
diff -Npru linux-2.6.10/fs/namei.c linux-2.6.10-audit/fs/namei.c
--- linux-2.6.10/fs/namei.c 2004-12-24 15:34:30.000000000 -0600
+++ linux-2.6.10-audit/fs/namei.c 2005-01-18 13:03:18.000000000 -0600
@@ -232,6 +232,10 @@ int permission(struct inode * inode,int
/* Ordinary permission routines do not understand MAY_APPEND. */
submask = mask & ~MAY_APPEND;
+ retval = audit_notify_watch(current, inode, mask);
+ if (retval < 0)
+ return retval;
I wouldn't expect this to fail.
if (inode->i_op && inode->i_op->permission)
retval = inode->i_op->permission(inode, submask, nd);
else
diff -Npru linux-2.6.10/include/linux/audit.h linux-2.6.10-audit/include/linux/audit.h
--- linux-2.6.10/include/linux/audit.h 2005-01-19 15:18:52.000000000 -0600
+++ linux-2.6.10-audit/include/linux/audit.h 2005-01-19 15:37:56.000000000 -0600
@@ -25,14 +25,16 @@
#define _LINUX_AUDIT_H_
/* Request and reply types */
-#define AUDIT_GET 1000 /* Get status */
-#define AUDIT_SET 1001 /* Set status (enable/disable/auditd) */
-#define AUDIT_LIST 1002 /* List filtering rules */
-#define AUDIT_ADD 1003 /* Add filtering rule */
-#define AUDIT_DEL 1004 /* Delete filtering rule */
-#define AUDIT_USER 1005 /* Send a message from user-space */
-#define AUDIT_LOGIN 1006 /* Define the login id and informaiton */
-#define AUDIT_KERNEL 2000 /* Asynchronous audit record. NOT A REQUEST. */
+#define AUDIT_GET 1000 /* Get status */
+#define AUDIT_SET 1001 /* Set status (enable/disable/auditd) */
+#define AUDIT_LIST 1002 /* List filtering rules */
+#define AUDIT_ADD 1003 /* Add filtering rule */
+#define AUDIT_DEL 1004 /* Delete filtering rule */
+#define AUDIT_USER 1005 /* Send a message from user-space */
+#define AUDIT_LOGIN 1006 /* Define the login id and information */
+#define AUDIT_WATCH_INS 1007 /* Insert file/dir watch entry */
+#define AUDIT_WATCH_REM 1008 /* Remove file/dir watch entry */
+#define AUDIT_KERNEL 2000 /* Asynchronous audit record. NOT A REQUEST. */
/* Rule flags */
#define AUDIT_PER_TASK 0x01 /* Apply rule at task creation (not syscall) */
@@ -74,7 +76,7 @@
#define AUDIT_DEVMINOR 101
#define AUDIT_INODE 102
#define AUDIT_EXIT 103
-#define AUDIT_SUCCESS 104 /* exit >= 0; value ignored */
+#define AUDIT_SUCCESS 104 /* exit >= 0; value ignored */
#define AUDIT_ARG0 200
#define AUDIT_ARG1 (AUDIT_ARG0+1)
@@ -129,6 +131,31 @@ struct audit_rule { /* for AUDIT_LIST,
__u32 values[AUDIT_MAX_FIELDS];
};
+/* Input structure for fs object auditing */
+
+struct audit_watch_request {
+ char *path;
+ char *fk;
s/fk/filterkey/?
+ int pathlen;
+ int fklen;
+};
+
+/* Watch entry */
+
+/* Following two structs must be commented out for userspace copy */
+
+struct audit_watch {
+ char *name;
+ char *fk;
+ struct list_head list;
+};
Anyway to come up with name distintion between this data and the
function? And with many of these at 16bytes each on 32bit, it's a large
waste (smallest kmalloc slab is 32bytes iirc)
+struct audit_data {
+ struct audit_watch *watch;
+ struct list_head watchlist;
+};
Does audit_watch ever exist without audit_data? Looking through, it
seems like they can be decoupled, but that's only on rename, where I
think you wouldn't want to change, or drop the whole thing and start
over.
@@ -155,6 +182,8 @@ extern int audit_receive_filter(int typ
extern void audit_get_stamp(struct audit_context *ctx,
struct timespec *t, int *serial);
extern int audit_set_loginuid(struct audit_context *ctx, uid_t loginuid);
+extern int audit_notify_watch(struct task_struct *task, struct inode *inode,
+ int mask);
#else
#define audit_alloc(t) ({ 0; })
#define audit_free(t) do { ; } while (0)
@@ -163,8 +192,22 @@ extern int audit_set_loginuid(struct au
#define audit_getname(n) do { ; } while (0)
#define audit_putname(n) do { ; } while (0)
#define audit_inode(n,i,d) do { ; } while (0)
+#define audit_notify_watch(t,i,m) do { ; } while(0)
#endif
+#ifdef CONFIG_AUDITFILESYSTEM
+extern int audit_receive_watch(int type, int pid, int uid, int seq,
+ struct audit_watch_request *req);
+extern int audit_watch(struct dentry *dentry);
+extern void audit_inode_free(struct inode *inode);
+extern struct audit_data *audit_inode_alloc(void);
+#else
+#define audit_receive_watch(t,p,u,s,r) do { ; } while(0)
+#define audit_watch(d) do { ; } while(0)
+#define audit_inode_free(i) do { ; } while(0)
+#define audit_inode_alloc() do { ; } while(0)
+#endif
+
#ifdef CONFIG_AUDIT
/* These are defined in audit.c */
/* Public API */
diff -Npru linux-2.6.10/include/linux/fs.h linux-2.6.10-audit/include/linux/fs.h
--- linux-2.6.10/include/linux/fs.h 2004-12-24 15:34:27.000000000 -0600
+++ linux-2.6.10-audit/include/linux/fs.h 2005-01-07 16:17:20.000000000 -0600
@@ -27,6 +27,7 @@ struct poll_table_struct;
struct kstatfs;
struct vm_area_struct;
struct vfsmount;
+struct audit_data;
/*
* It's silly to have NR_OPEN bigger than NR_FILE, but you can change
@@ -459,6 +460,7 @@ struct inode {
#ifdef CONFIG_QUOTA
struct dquot *i_dquot[MAXQUOTAS];
#endif
+ struct audit_data *i_audit;
/* These three should probably be a union */
struct list_head i_devices;
struct pipe_inode_info *i_pipe;
diff -Npru linux-2.6.10/init/Kconfig linux-2.6.10-audit/init/Kconfig
--- linux-2.6.10/init/Kconfig 2004-12-24 15:35:24.000000000 -0600
+++ linux-2.6.10-audit/init/Kconfig 2005-01-16 22:35:08.000000000 -0600
@@ -174,6 +174,17 @@ config AUDITSYSCALL
can be used independently or with another kernel subsystem,
such as SELinux.
+config AUDITFILESYSTEM
+ bool "Enable filesystem auditing support"
+ depends on AUDITSYSCALL
+ default n
+ help
+ Generate audit records for regular files or directories that
+ are being watched for access by audited syscalls. To insert
+ and remove watch points into the filesystem you may use the
+ auditctl program provided with auditd. For more information,
+ 'man auditctl'
+
config LOG_BUF_SHIFT
int "Kernel log buffer size (16 => 64KB, 17 => 128KB)" if DEBUG_KERNEL
range 12 21
diff -Npru linux-2.6.10/kernel/Makefile linux-2.6.10-audit/kernel/Makefile
--- linux-2.6.10/kernel/Makefile 2004-12-24 15:34:26.000000000 -0600
+++ linux-2.6.10-audit/kernel/Makefile 2005-01-05 15:23:14.000000000 -0600
@@ -23,6 +23,7 @@ obj-$(CONFIG_IKCONFIG_PROC) += configs.o
obj-$(CONFIG_STOP_MACHINE) += stop_machine.o
obj-$(CONFIG_AUDIT) += audit.o
obj-$(CONFIG_AUDITSYSCALL) += auditsc.o
+obj-$(CONFIG_AUDITFILESYSTEM) += auditfs.o
obj-$(CONFIG_KPROBES) += kprobes.o
obj-$(CONFIG_SYSFS) += ksysfs.o
obj-$(CONFIG_GENERIC_HARDIRQS) += irq/
diff -Npru linux-2.6.10/kernel/audit.c linux-2.6.10-audit/kernel/audit.c
--- linux-2.6.10/kernel/audit.c 2004-12-24 15:35:50.000000000 -0600
+++ linux-2.6.10-audit/kernel/audit.c 2005-01-19 11:30:02.000000000 -0600
@@ -20,6 +20,7 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*
* Written by Rickard E. (Rik) Faith <faith at redhat.com>
+ * Modified by Timothy R. Chavez <chavezt at us.ibm.com>
*
* Goals: 1) Integrate fully with SELinux.
* 2) Minimal run-time overhead:
@@ -46,7 +47,6 @@
#include <asm/types.h>
#include <linux/mm.h>
#include <linux/module.h>
-
#include <linux/audit.h>
#include <net/sock.h>
@@ -60,6 +60,9 @@ static int audit_initialized;
/* No syscall auditing will take place unless audit_enabled != 0. */
int audit_enabled;
+/* No filesystem auditing will take place unless audit_filesystem != 0 */
+int audit_filesystem;
Err, this is only set based on config. So it doesn't really do
anything.
+
/* Default state when kernel boots without any parameters. */
static int audit_default;
@@ -394,6 +397,15 @@ static int audit_receive_msg(struct sk_b
err = -EOPNOTSUPP;
#endif
break;
+#ifdef CONFIG_AUDITFILESYSTEM
+ case AUDIT_WATCH_INS:
+ case AUDIT_WATCH_REM:
+ err = audit_receive_watch(nlh->nlmsg_type, pid, uid, seq,
+ data);
+#else
+ err = -EOPNOTSUPP;
+#endif
+ break;
default:
err = -EINVAL;
break;
@@ -533,6 +545,16 @@ int __init audit_init(void)
audit_initialized = 1;
audit_enabled = audit_default;
+/* FIXME:
+ * We should be able to enable/disable filesystem auditing dynamically,
+ * after we boot up. Configuring support for it should != automatic
+ * enablement.
+ */
+#ifdef CONFIG_AUDITFILESYSTEM
+ audit_filesystem = 1;
+#else
+ audit_filesystem = 0;
+#endif
Oh, nm, I see the fixme comment ;-)
audit_log(NULL, "initialized");
return 0;
}
diff -Npru linux-2.6.10/kernel/auditfs.c linux-2.6.10-audit/kernel/auditfs.c
--- linux-2.6.10/kernel/auditfs.c 1969-12-31 17:00:00.000000000 -0700
+++ linux-2.6.10-audit/kernel/auditfs.c 2005-01-19 14:48:08.000000000 -0600
@@ -0,0 +1,396 @@
+/* auditfs.c -- Filesystem auditing support -*- linux-c -*-
+ * Implements filesystem auditing support, depends on kernel/auditsc.c
+ *
+ * Copyright 2005 International Business Machines Corp. (IBM)
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
+ *
+ * Written by Timothy R. Chavez <chavezt at us.ibm.com>
+ *
+ */
+
+#include <linux/init.h>
+
+#include <linux/fs.h>
+#include <linux/sched.h>
+#include <linux/kernel.h>
+#include <linux/namei.h>
+#include <linux/mount.h>
+#include <linux/list.h>
+#include <linux/mm.h>
+#include <linux/audit.h>
+
+#include <asm/types.h>
+#include <asm/uaccess.h>
+
+/* Helper functions */
+
+struct audit_lookup {
+ char *name;
+ struct dentry *dentry;
+ struct dentry *parent;
+};
+
+/* audit_path_lookup - Look up a string path and store the parent dentry,
+ * string name of the file/dir we're interested in,
+ * and optionally, the dentry of the file/dir we're
+ * interested in if it exists
+ *
+ * @path - String path (ie: "/etc/passwd")
+ * @lup - Lookup structure
These aren't proper docbook style comments.
+ *
+ * NOTE: caller must dput dentries if necessary
+ */
+static int audit_path_lookup(const char *path, struct audit_lookup *lup)
+{
+ int ret;
+ struct nameidata nd;
+
+ ret = path_lookup(path, LOOKUP_PARENT | LOOKUP_FOLLOW, &nd);
+ if (ret < 0)
+ goto audit_path_lookup_exit;
+
+ lup->parent = dget(nd.dentry);
+ lup->dentry = d_lookup(nd.dentry, &nd.last);
+ lup->name = kmalloc(strlen(nd.last.name) + 1, GFP_KERNEL);
+ if (!lup->name)
+ return -ENOMEM;
Never called path_release()
+ strcpy(lup->name, nd.last.name);
+
+ path_release(&nd);
+
+ audit_path_lookup_exit:
+ return ret;
+}
+
+/* audit_fetch_watch - If the string name matches that of
+ * of a watch entry in the parent dentry
+ * then return that watch structure.
+ *
+ * @name - String name (ie: "passwd")
+ * @parent - The dentry of the inode whose watchlist
+ we want to traverse
+ */
DocBook comments here too, I think they all need to be redone.
+static struct audit_watch *audit_fetch_watch(const char *name,
+ struct dentry *parent)
+{
+ struct audit_data *data;
+ struct audit_watch *watch, *ret = NULL;
+
+ data = parent->d_inode->i_audit;
+ if (!data)
+ goto audit_fetch_watch_exit;
+
+ if (list_empty(&data->watchlist))
+ goto audit_fetch_watch_exit;
list_for_each will tell you if it's empty.
+ list_for_each_entry(watch, &data->watchlist, list)
+ if (!strcmp(watch->name, name)) {
+ ret = watch;
+ goto audit_fetch_watch_exit;
usually just break;
+ }
Have a feel for how many are usually in a list? for /etc, for example,
maybe a hash is better?
+ audit_fetch_watch_exit:
+ return ret;
+}
+
+static struct audit_watch *audit_watch_alloc(void)
+{
+ struct audit_watch *watch;
+
+ watch = kmalloc(sizeof(struct audit_watch), GFP_KERNEL);
+ if (!watch)
+ return NULL;
+
+ watch->name = NULL;
+ watch->fk = NULL;
+
I'd do:
if (watch) {
watch->name = NULL;
watch->fk = NULL;
}
+ return watch;
To get a single point of return. Most places that are leaking would
benefit from this type of coding style.
+}
+
+static void audit_watch_free(struct audit_watch *watch)
+{
+ if (watch) {
+ if (watch->name)
+ kfree(watch->name);
+ if (watch->fk)
+ kfree(watch->fk);
kfree(NULL) is fine.
+ kfree(watch);
+ }
+}
+
+/* audit_create_watch - Add a watch entry to the dentry's parent's
+ * watchlist with the given name and
+ * fk.
+ *
+ * @name - String name (ie: "passwd")
+ * @fk - String fk (ie: "fk_file_passwd")
+ * @parent - The dentry of the inode with relevant
+ * watchlist
+ */
+
+static int
+audit_create_watch(const char *name, const char *fk, struct dentry *parent)
+{
+ struct audit_watch *watch;
+
+ /* FIXME: More appropriate errno for something that already exists? */
+ watch = audit_fetch_watch(name, parent);
+ if (watch)
+ return -EINVAL;
EEXIST? EBUSY?
+
+ if (!parent->d_inode->i_audit) {
+ parent->d_inode->i_audit = audit_inode_alloc();
+ if (!parent->d_inode->i_audit)
+ return -ENOMEM;
+ }
+
+ watch = audit_watch_alloc();
+ if (!watch)
+ return -ENOMEM;
Possible memory leak.
+ watch->name = kmalloc(strlen(name) + 1, GFP_KERNEL);
+ if (!watch->name)
+ return -ENOMEM;
+ watch->fk = kmalloc(strlen(fk) + 1, GFP_KERNEL);
+ if (!watch->fk)
+ return -ENOMEM;
More possible memory leaks.
+ strcpy(watch->name, name);
+ strcpy(watch->fk, fk);
+
+ list_add(&watch->list, &parent->d_inode->i_audit->watchlist);
We won't go into the locking ;-)
+ return 0;
+}
+
+/* audit_destroy_watch - Find the watch entry and then physically
+ * destroy it.
+ *
+ * @name - String name (ie: "passwd")
+ * @parent - The dentry of the inode with relevant
+ * watchlist
+ */
+
+static int audit_destroy_watch(const char *name, struct dentry *parent)
+{
+ struct audit_watch *watch;
+
+ /* FIXME: More appropriate errno for something that does not exist? */
+ watch = audit_fetch_watch(name, parent);
+ if (!watch)
+ return -EINVAL;
-EEXIST?
+
+ list_del_init(&watch->list);
+ audit_watch_free(watch);
+
+ if (!parent->d_inode->i_audit->watch &&
+ list_empty(&parent->d_inode->i_audit->watchlist))
+ audit_inode_free(parent->d_inode);
+
+ return 0;
+}
+
+/* audit_insert_watch - Insert a watch entry for req->name
+ * with req->fk into the filesystem.
+ *
+ * @req - The request originating from userspace.
+ *
+ * (Called by audit_receive_watch())
+ */
+
+static int audit_insert_watch(struct audit_watch_request *req)
+{
+ int ret;
+ char *fk, *path;
+ struct audit_lookup lup;
Why can't you just use nameidata?
+ path = kmalloc(req->pathlen, GFP_KERNEL);
+ if (!path)
+ return -ENOMEM;
+ fk = kmalloc(req->fklen, GFP_KERNEL);
+ if (!fk)
+ return -ENOMEM;
Possible memleak.
+ ret = strncpy_from_user(path, req->path, req->pathlen);
+ if (ret < 0)
+ return ret;
+ ret = strncpy_from_user(fk, req->fk, req->fklen);
+ if (ret < 0)
+ return ret;
req->pathlen is user specified. it can't overflow path, but should
these be checked against valid values, say PATH_MAX?
+ ret = audit_path_lookup(path, &lup);
+ if (ret < 0)
+ goto audit_insert_watch_exit;
+
+ ret = audit_create_watch(lup.name, fk, lup.parent);
+ if (ret < 0)
+ goto audit_insert_watch_exit;
+
+ ret = audit_watch(lup.dentry);
+
+ audit_insert_watch_exit:
CodingStyle, no tab before label.
+ if (lup.parent)
+ dput(lup.parent);
+ if (lup.dentry)
+ dput(lup.dentry);
+ return ret;
Never kfree path, fk, or lup.name.
+}
+
+/* audit_remove_watch - Remove a watch entry for req->name
+ * from the filesystem.
+ *
+ * @req - The request originating from userspace.
+ *
+ * (Called by audit_watch_receive())
+ */
+
+static int audit_remove_watch(struct audit_watch_request *req)
+{
+ int ret;
+ char *path;
+ struct audit_lookup lup;
Why not use nameidata?
+ path = kmalloc(req->pathlen, GFP_KERNEL);
+ if (!path)
+ return -ENOMEM;
getname()?
+ ret = strncpy_from_user(path, req->path, req->pathlen);
+ if (ret < 0)
+ goto audit_remove_watch_exit;
+
+ ret = audit_path_lookup(req->path, &lup);
That's the userspace pointer, should pass path.
+ if (ret < 0)
+ goto audit_remove_watch_exit;
+
+ ret = audit_destroy_watch(lup.name, lup.parent);
+ if (ret < 0)
+ goto audit_remove_watch_exit;
+
+ if (!lup.dentry)
+ goto audit_remove_watch_exit;
+
+ /* Free up inode audit data, if possible */
+ lup.dentry->d_inode->i_audit->watch = NULL;
audit_inode_free() does this too. is this safe? in audit_inode_free
you check against i_audit == NULL.
+ if (list_empty(&lup.dentry->d_inode->i_audit->watchlist))
+ audit_inode_free(lup.dentry->d_inode);
+
+ audit_remove_watch_exit:
CodingStyle, no tab before label.
+ if (lup.parent)
+ dput(lup.parent);
+ if (lup.dentry)
+ dput(lup.dentry);
+ return ret;
You never kfree path or lup.name.
+}
+
+/* Public interface */
+
+struct audit_data *audit_inode_alloc(void)
+{
+ struct audit_data *data;
+
+ data = kmalloc(sizeof(struct audit_data), GFP_KERNEL);
+ if (!data)
+ return NULL;
+
+ data->watch = NULL;
+ INIT_LIST_HEAD(&data->watchlist);
+
+ return data;
+}
+
+void audit_inode_free(struct inode *inode)
+{
+ struct audit_watch *watch, *tmp;
+
+ if (!inode->i_audit)
+ return;
+
+ inode->i_audit->watch = NULL;
+ list_for_each_entry_safe(watch, tmp, &inode->i_audit->watchlist,
+ list) {
+ list_del(&watch->list);
+ audit_watch_free(watch);
+ }
+
+ kfree(inode->i_audit);
+ inode->i_audit = NULL;
+}
+
+/* If child exists in parent's watchlist, the child will point
+ * into the parent's watchlist at its own entry. Otherwise,
+ * it will point to NULL
+ *
+ * Hook appears in: fs/dcache.c:d_instantiate(), d_move(), and d_splice_alias()
+ */
+int audit_watch(struct dentry *dentry)
+{
+ struct audit_watch *watch;
+
+ /* NOTE: Not being watched != error */
+ if (!dentry || !dentry->d_inode)
+ return 0;
+
+ watch = audit_fetch_watch(dentry->d_name.name, dentry->d_parent);
+ if (!watch)
+ return 0;
+
+ /* In the event that we're being watched, but not yet allocated. */
+ if (!dentry->d_inode->i_audit) {
+ dentry->d_inode->i_audit = audit_inode_alloc();
+ if (!dentry->d_inode->i_audit)
+ return -ENOMEM;
+ }
+ dentry->d_inode->i_audit->watch = watch;
+
+ return 0;
+}
+
+/* Called from kernel/audit.c by audit_receive_msg()
+ *
+ * Code path for watch insertions:
+ * audit_receive_watch()->audit_insert_watch()->audit_create_watch()
+ * Code path for watch removals:
+ * audit_receive_watch()->audit_remove_watch()->audit_destroy_watch()
+ *
+ */
+int
+audit_receive_watch(int type, int pid, int uid, int seq,
+ struct audit_watch_request *req)
+{
+ int ret;
+
+ switch (type) {
+ case AUDIT_WATCH_INS:
+ ret = audit_insert_watch(req);
+ break;
+ case AUDIT_WATCH_REM:
+ ret = audit_remove_watch(req);
+ break;
+ default:
+ return -EINVAL;
do you want to reply to userspace here or below on error?
+ }
+
+ /* Need to return something more meaningful to userspace and
+ * update comment that states audit_send_reply is only called
+ * from auditsc.c
+ */
+ if (!ret)
+ audit_send_reply(pid, seq, type, 0, 1, NULL, 0);
+
+ return ret;
+}
diff -Npru linux-2.6.10/kernel/auditsc.c linux-2.6.10-audit/kernel/auditsc.c
--- linux-2.6.10/kernel/auditsc.c 2004-12-24 15:35:24.000000000 -0600
+++ linux-2.6.10-audit/kernel/auditsc.c 2005-01-19 15:39:22.000000000 -0600
@@ -19,6 +19,7 @@
* Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
*
* Written by Rickard E. (Rik) Faith <faith at redhat.com>
+ * Modified by Timothy R. Chavez <chavezt at us.ibm.com>
*
* Many of the ideas implemented here are from Stephen C. Tweedie,
* especially the idea of avoiding a copy by using getname.
@@ -49,6 +50,11 @@
/* No syscall auditing will take place unless audit_enabled != 0. */
extern int audit_enabled;
+/* No syscall will collect information about auditable file/dirs
+ * unless audit_filesystem != 0
+ */
+extern int audit_filesystem;
+
/* AUDIT_NAMES is the number of slots we reserve in the audit_context
* for saving names from getname(). */
#define AUDIT_NAMES 20
@@ -113,6 +119,7 @@ struct audit_context {
uid_t uid, euid, suid, fsuid;
gid_t gid, egid, sgid, fsgid;
unsigned long personality;
+ struct list_head watched; /* The auditable files/dirs accessed */
#if AUDIT_DEBUG
int put_count;
@@ -134,6 +141,22 @@ struct audit_entry {
struct audit_rule rule;
};
+/* The structure that stores information about files/directories being
+ * watched in the filesystem, that the syscall accessed.
+ */
+
+struct audit_file {
+ char *name;
+ char *fk;
+ unsigned long ino;
+ umode_t mode;
+ uid_t uid;
+ gid_t gid;
+ dev_t rdev;
+ int mask;
+ struct list_head list;
+};
+
/* Check to see if two rules are identical. It is called from
* audit_del_rule during AUDIT_DEL. */
static int audit_compare_rule(struct audit_rule *a, struct audit_rule *b)
@@ -506,6 +529,44 @@ static inline void audit_free_names(stru
context->name_count = 0;
}
+static struct audit_file *audit_file_alloc(void)
+{
+ struct audit_file *file;
+
+ file = kmalloc(sizeof(struct audit_file), GFP_KERNEL);
+ if (!file)
+ return NULL;
+
+ file->name = NULL;
+ file->fk = NULL;
+
+ return file;
single point of return.
+}
+
+static void audit_file_free(struct audit_file *file)
+{
+ if (file) {
+ if (file->name)
+ kfree(file->name);
+ if (file->fk)
+ kfree(file->fk);
kfree(NULL) ok.
+ kfree(file);
+ }
+}
+
+static inline void audit_free_files(struct audit_context *context)
+{
+ struct audit_file *file, *tmp;
+
+ if (!audit_filesystem)
+ return;
I don't think that's right. If it was dynamically turned off (after
fixme is fixed) then this could leak some already created audit_files,
right?
+
+ list_for_each_entry_safe(file, tmp, &context->watched, list) {
+ list_del(&file->list);
+ audit_file_free(file);
+ }
+}
+
static inline void audit_zero_context(struct audit_context *context,
enum audit_state state)
{
@@ -514,6 +575,7 @@ static inline void audit_zero_context(st
memset(context, 0, sizeof(*context));
context->state = state;
context->loginuid = loginuid;
+ INIT_LIST_HEAD(&context->watched);
}
static inline struct audit_context *audit_alloc_context(enum audit_state state)
@@ -572,6 +634,7 @@ static inline void audit_free_context(st
context->name_count, count);
}
audit_free_names(context);
+ audit_free_files(context);
kfree(context);
context = previous;
} while (context);
@@ -582,6 +645,7 @@ static inline void audit_free_context(st
static void audit_log_exit(struct audit_context *context)
{
int i;
+ struct audit_file *file;
struct audit_buffer *ab;
ab = audit_log_start(context);
@@ -628,6 +692,24 @@ static void audit_log_exit(struct audit_
MINOR(context->names[i].rdev));
audit_log_end(ab);
}
+
+ if (!audit_filesystem)
+ return;
+ list_for_each_entry(file, &context->watched, list) {
+ ab = audit_log_start(context);
+ if (!ab)
+ continue;
+
+ /* Do we need more information? */
+ audit_log_format(ab,
+ "name=%s filter_key=%s inode=%lu inode_mode=%d "
+ "inode_uid=%d inode_gid=%d inode_dev=%02x:%02x "
+ "mask=%d",
+ file->name, file->fk, file->ino, file->mode,
+ file->uid, file->gid, MAJOR(file->rdev),
+ MINOR(file->rdev), file->mask);
+ audit_log_end(ab);
+ }
}
/* Free a per-task audit context. Called from copy_process and
@@ -791,6 +873,7 @@ void audit_syscall_exit(struct task_stru
tsk->audit_context = new_context;
} else {
audit_free_names(context);
+ audit_free_files(context);
audit_zero_context(context, context->state);
tsk->audit_context = context;
}
@@ -914,3 +997,58 @@ int audit_set_loginuid(struct audit_cont
}
return 0;
}
+
+/* If file/dir has an audit_context and has filesystem auditing
+ * turned on, then if this inode is being watched, collect info
+ * about it.
+ *
+ * Hook located in: fs/namie.c:permission()
+ */
+
+int audit_notify_watch(struct task_struct *task, struct inode *inode,
+ int mask)
+{
+ struct audit_context *context;
+ struct audit_file *file;
+
+ context = current->audit_context;
Technically, this is a bug. You pass the task, but use current.
Luckily, this only appears to be called in current context.
+ /* Do we have a context and are we in a syscall? */
+ if (!context || !context->in_syscall)
+ return 0;
+
+ /* Do we care about file system auditing? */
+ if (!audit_filesystem)
+ return 0;
+
+ /* Are we being watched? */
+ if (!inode->i_audit || !inode->i_audit->watch)
+ return 0;
+
+ file = audit_file_alloc();
+ if (!file)
+ return -ENOMEM;
+
+ file->name = kmalloc(strlen(inode->i_audit->watch->name) + 1,
+ GFP_KERNEL);
+ if (!file->name)
+ return -ENOMEM;
+
+ file->fk = kmalloc(strlen(inode->i_audit->watch->fk) + 1,
+ GFP_KERNEL);
+ if (!file->fk)
+ return -ENOMEM;
Anyway to avoid these extra allocations (which leak on failure paths ;-)?
Like refcount the watch struct and pass that along? Ditto for the inode
data.
+ strcpy(file->name, inode->i_audit->watch->name);
+ strcpy(file->fk, inode->i_audit->watch->fk);
+ file->ino = inode->i_ino;
+ file->uid = inode->i_uid;
+ file->gid = inode->i_gid;
+ file->rdev = inode->i_rdev;
+ file->mask = mask;
+
+ list_add(&file->list, &task->audit_context->watched);
+
+ return 0;
+}
+
More information about the Linux-audit
mailing list