8:23 p.m.
Hello,
For some reason I thought I had updated to .48, but no matter methinks, these
updates should apply to .48 as well.
-tim
8:35 p.m.
On Tuesday 24 May 2005 20:23, Timothy R. Chavez wrote:
Hello,
For some reason I thought I had updated to .48, but no matter methinks,
these
updates should apply to .48 as well.
-tim
Oops, looks like I missed a couple places where there were still spaces...
here's an update (apply this one instead of update1).
-tim
8:46 a.m.
On Tue, 2005-05-24 at 20:35 -0500, Timothy R. Chavez wrote:
Oops, looks like I missed a couple places where there were still
spaces... here's an update (apply this one instead of update1).
You missed some more too. But what you sent me is building in audit.49
now anyway.
Why is audit_notify_watch() in auditfs.c? Can it ever do anything useful
if !CONFIG_AUDITFILESYSTEM?
--
dwmw2
9:11 a.m.
On Wed, 2005-05-25 at 14:46 +0100, David Woodhouse wrote:
Why is audit_notify_watch() in auditfs.c? Can it ever do anything
useful if !CONFIG_AUDITFILESYSTEM?
Hm, because it needs visibility to the audit_context. Could possibly be
split into two functions though, along the following lines...
As an added bonus, this should prevent it oopsing if a task doesn't have
an audit context but touches a watched inode.
Are we really supposed to be using GFP_KERNEL in audit_notify_watch()
(now in audit_attach_wdata()) ? We're never checking the return value
from audit_notify_watch() either.
--- linux-2.6.9/include/linux/audit.h~ 2005-05-25 14:30:18.000000000 +0100
+++ linux-2.6.9/include/linux/audit.h 2005-05-25 15:05:00.000000000 +0100
@@ -277,7 +277,6 @@ extern int audit_socketcall(int nargs, u
extern int audit_sockaddr(int len, void *addr);
extern int audit_avc_path(struct dentry *dentry, struct vfsmount *mnt);
extern void audit_signal_info(int sig, struct task_struct *t);
-extern int audit_notify_watch(struct inode *inode, int mask);
#else
#define audit_alloc(t) ({ 0; })
#define audit_free(t) do { ; } while (0)
@@ -294,7 +293,6 @@ extern int audit_notify_watch(struct ino
#define audit_sockaddr(len, addr) ({ 0; })
#define audit_avc_path(dentry, mnt) ({ 0; })
#define audit_signal_info(s,t) do { ; } while (0)
-#define audit_notify_watch(i,m) ({ 0; })
#endif
#ifdef CONFIG_AUDITFILESYSTEM
@@ -308,6 +306,9 @@ extern void audit_update_watch(struct de
extern void audit_wentry_put(struct audit_wentry *wentry);
extern void audit_dentry_unpin(struct dentry *dentry);
extern struct audit_wentry *audit_wentry_get(struct audit_wentry *wentry);
+extern int audit_notify_watch(struct inode *inode, int mask);
+extern int auditfs_attach_wdata(struct inode *inode, struct audit_wentry *wentry,
+ int mask);
#else
#define audit_filesystem_init() ({ 0; })
#define audit_list_watches(p,s) ({ -EOPNOTSUPP; })
@@ -317,6 +318,7 @@ extern struct audit_wentry *audit_wentry
#define audit_update_watch(d,r) do { ; } while (0)
#define audit_wentry_put(w) do { ; } while(0)
#define audit_wentry_get(w) ({ 0; })
+#define audit_notify_watch(i,m) ({ 0; })
#endif
#ifdef CONFIG_AUDIT
--- linux-2.6.9/kernel/auditfs.c~ 2005-05-25 14:30:18.000000000 +0100
+++ linux-2.6.9/kernel/auditfs.c 2005-05-25 15:06:49.000000000 +0100
@@ -35,6 +35,8 @@
#include <asm/uaccess.h>
+#define inode_audit_data(inode) ((inode)->i_audit)
+
extern int audit_enabled;
static kmem_cache_t *audit_watch_cache;
@@ -409,7 +411,7 @@ static inline int audit_insert_watch(str
watch->filterkey,
watch->perms,
nd.dentry->d_inode->i_sb->s_dev,
- nd.dentry->d_inode->i_audit);
+ inode_audit_data(nd.dentry->d_inode));
if (ret < 0)
goto audit_insert_watch_release;
@@ -444,7 +446,7 @@ static inline int audit_remove_watch(str
if (nd.last_type != LAST_NORM || !nd.last.name)
goto audit_remove_watch_release;
- data = nd.dentry->d_inode->i_audit;
+ data = inode_audit_data(nd.dentry->d_inode);
write_lock(&data->lock);
wentry = audit_wentry_fetch(nd.last.name, data);
@@ -508,8 +510,8 @@ void audit_update_watch(struct dentry *d
if (!dentry->d_parent || !dentry->d_parent->d_inode)
return;
- data = dentry->d_inode->i_audit;
- parent = dentry->d_parent->d_inode->i_audit;
+ data = inode_audit_data(dentry->d_inode);
+ parent = inode_audit_data(dentry->d_parent->d_inode);
wentry = audit_wentry_fetch_lock(dentry->d_name.name, parent);
@@ -657,8 +659,8 @@ audit_receive_watch_exit:
int audit_inode_alloc(struct inode *inode)
{
if (inode) {
- inode->i_audit = audit_data_alloc();
- if (!inode->i_audit)
+ inode_audit_data(inode) = audit_data_alloc();
+ if (!inode_audit_data(inode))
return ENOMEM;
}
@@ -668,7 +670,7 @@ int audit_inode_alloc(struct inode *inod
void audit_inode_free(struct inode *inode)
{
if (inode)
- audit_data_free(inode->i_audit);
+ audit_data_free(inode_audit_data(inode));
}
/*
* When we delete a dentry we check to see whether or not we're being
@@ -682,10 +684,12 @@ void audit_dentry_unpin(struct dentry *d
struct audit_inode_data *parent;
struct audit_wentry *wentry;
- parent = dentry->d_parent->d_inode->i_audit;
+ parent = inode_audit_data(dentry->d_parent->d_inode);
wentry = audit_wentry_fetch_lock(dentry->d_name.name, parent);
if (wentry) {
- audit_unpin(dentry->d_inode->i_audit->wentry);
+ struct audit_inode_data *data = inode_audit_data(dentry->d_inode);
+
+ audit_unpin(data->wentry);
audit_wentry_put(wentry);
}
}
@@ -716,3 +720,43 @@ audit_filesystem_init_fail:
audit_filesystem_init_exit:
return ret;
}
+
+
+int audit_notify_watch(struct inode *inode, int mask)
+{
+ int ret = 0;
+ struct audit_inode_data *data;
+ struct audit_wentry *wentry = NULL;
+
+ if (likely(!audit_enabled))
+ return 0;
+
+ if (!inode)
+ return 0;
+
+ data = inode_audit_data(inode);
+ if (!data)
+ return 0;
+
+ /*
+ * Won't be able to drop i_audit->wentry reference
+ * before we obtain our own reference
+ */
+ read_lock(&data->lock);
+ wentry = audit_wentry_get(data->wentry);
+ read_unlock(&data->lock);
+ if (!wentry)
+ return 0;
+
+ if (mask && (wentry->w_watch->perms &&
!(wentry->w_watch->perms&mask)))
+ goto audit_notify_watch_fail;
+
+ ret = auditfs_attach_wdata(inode, wentry, mask);
+ if (!ret)
+ return 0;
+
+ audit_notify_watch_fail:
+ audit_wentry_put(wentry);
+
+ return ret;
+}
--- linux-2.6.9/kernel/auditsc.c~ 2005-05-25 14:30:18.000000000 +0100
+++ linux-2.6.9/kernel/auditsc.c 2005-05-25 15:06:25.000000000 +0100
@@ -1180,41 +1180,19 @@ void audit_signal_info(int sig, struct t
}
}
-
-int audit_notify_watch(struct inode *inode, int mask)
+#ifdef CONFIG_AUDITFILESYSTEM
+/* This has to be here instead of in auditfs.c, because it needs to
+ see the audit context */
+int auditfs_attach_wdata(struct inode *inode, struct audit_wentry *wentry,
+ int mask)
{
- int ret = 0;
struct audit_context *context = current->audit_context;
struct audit_aux_data_watched *ax;
- struct audit_wentry *wentry = NULL;
- if (likely(!audit_enabled))
- goto audit_notify_watch_exit;
-
- if (!inode || !inode->i_audit)
- goto audit_notify_watch_exit;
-
- /*
- * Won't be able to drop i_audit->wentry reference
- * before we obtain our own reference
- */
- read_lock(&inode->i_audit->lock);
- wentry = audit_wentry_get(inode->i_audit->wentry);
- if (!wentry) {
- read_unlock(&inode->i_audit->lock);
- goto audit_notify_watch_exit;
- }
- read_unlock(&inode->i_audit->lock);
-
- if (mask && (wentry->w_watch->perms &&
!(wentry->w_watch->perms&mask)))
- goto audit_notify_watch_fail;
-
- ret = -ENOMEM;
ax = kmalloc(sizeof(*ax), GFP_KERNEL);
if (!ax)
- goto audit_notify_watch_fail;
+ return -ENOMEM;
- ret = 0;
if (context->in_syscall && !context->auditable)
context->auditable = 1;
@@ -1230,10 +1208,6 @@ int audit_notify_watch(struct inode *ino
ax->link.next = context->aux;
context->aux = (void *)ax;
- goto audit_notify_watch_exit;
-
-audit_notify_watch_fail:
- audit_wentry_put(wentry);
-audit_notify_watch_exit:
- return ret;
+ return 0;
}
+#endif /* CONFIG_AUDITFILESYSTEM */
--
dwmw2
10:31 a.m.
On Wednesday 25 May 2005 09:11, David Woodhouse wrote:
On Wed, 2005-05-25 at 14:46 +0100, David Woodhouse wrote:
> Why is audit_notify_watch() in auditfs.c? Can it ever do anything
> useful if !CONFIG_AUDITFILESYSTEM?
Hm, because it needs visibility to the audit_context. Could possibly be
split into two functions though, along the following lines...
As an added bonus, this should prevent it oopsing if a task doesn't have
an audit context but touches a watched inode.
Are we really supposed to be using GFP_KERNEL in audit_notify_watch()
(now in audit_attach_wdata()) ? We're never checking the return value
from audit_notify_watch() either.
I'll play with this. I thought that GFP_KERNEL was okay here. I think I'm
only ever under a semaphore if I'm under a lock at this point.
I like your little macro.
-tim
10:59 a.m.
On Wed, 2005-05-25 at 10:33 -0500, Timothy R. Chavez wrote:
> I like your little macro.
>
Yikes, that sounds a bit dirty. My bad. ;)
:)
What I'm doing with it _is_ a bit dirty. Looks something like this.
It compiles but I haven't tested it yet or even really done a second
read through the code -- I have to be home on time today :)
It's very loosely based on Serge's patch to use a hash table for all
security objects, although I think only the module_param() line actually
survives from the original, and even that can probably go too.
--- linux-2.6.9/include/linux/audit.h~ 2005-05-25 15:56:32.000000000 +0100
+++ linux-2.6.9/include/linux/audit.h 2005-05-25 16:28:08.000000000 +0100
@@ -215,6 +215,8 @@ struct watch_transport {
/* Structure associated with inode->i_audit */
struct audit_inode_data {
+ struct audit_inode_data *next_hash;
+ struct inode *inode;
struct audit_wentry *wentry;
struct hlist_head watchlist;
rwlock_t lock;
--- linux-2.6.9/include/linux/fs.h~ 2005-05-25 14:30:18.000000000 +0100
+++ linux-2.6.9/include/linux/fs.h 2005-05-25 16:46:27.000000000 +0100
@@ -462,7 +462,6 @@ struct inode {
unsigned long i_dnotify_mask; /* Directory notify events */
struct dnotify_struct *i_dnotify; /* for directory notifications */
- struct audit_inode_data *i_audit;
unsigned long i_state;
unsigned long dirtied_when; /* jiffies of first dirtying */
--- linux-2.6.9/fs/inode.c~ 2005-05-25 14:30:18.000000000 +0100
+++ linux-2.6.9/fs/inode.c 2005-05-25 16:47:35.000000000 +0100
@@ -135,7 +135,6 @@ static struct inode *alloc_inode(struct
inode->i_bdev = NULL;
inode->i_cdev = NULL;
inode->i_rdev = 0;
- inode->i_audit = NULL;
inode->i_security = NULL;
inode->dirtied_when = 0;
if (audit_inode_alloc(inode) || security_inode_alloc(inode)) {
--- linux-2.6.9/kernel/auditfs.c~ 2005-05-25 15:56:32.000000000 +0100
+++ linux-2.6.9/kernel/auditfs.c 2005-05-25 16:44:10.000000000 +0100
@@ -30,8 +30,10 @@
#include <linux/namei.h>
#include <linux/mount.h>
#include <linux/list.h>
+#include <linux/hash.h>
#include <linux/slab.h>
#include <linux/audit.h>
+#include <linux/module.h>
#include <asm/uaccess.h>
@@ -50,8 +52,32 @@ struct audit_skb_list {
size_t size;
};
-#define inode_audit_data(inode) ((inode)->i_audit)
+static struct audit_inode_data **auditfs_hash_table;
+static spinlock_t auditfs_hash_lock = SPIN_LOCK_UNLOCKED;
+static int auditfs_hash_bits;
+static int auditfs_cache_buckets = 16384;
+module_param(auditfs_cache_buckets, int, 0);
+MODULE_PARM_DESC(auditfs_cache_buckets, "Number of auditfs cache entries to allocate
(default 16384)\n");
+
+struct audit_inode_data *inode_audit_data(struct inode *inode)
+{
+ struct audit_inode_data **list;
+ int h = hash_ptr(inode, auditfs_hash_bits);
+ struct audit_inode_data *ret = NULL;
+
+ list = &auditfs_hash_table[h];
+ spin_lock(&auditfs_hash_lock);
+
+ while (*list && (unsigned long)((*list)->inode) < (unsigned long)inode)
+ list = &(*list)->next_hash;
+
+ if (*list && (*list)->inode == inode)
+ ret = *list;
+
+ spin_unlock(&auditfs_hash_lock);
+ return ret;
+}
/* Private Interface */
@@ -371,19 +397,6 @@ static inline void audit_drain_watchlist
audit_destroy_wentry(wentry);
}
-static inline struct audit_inode_data *audit_data_alloc(void)
-{
- struct audit_inode_data *data;
-
- data = kmalloc(sizeof(struct audit_inode_data), GFP_KERNEL);
- if (data) {
- data->wentry = NULL;
- INIT_HLIST_HEAD(&data->watchlist);
- data->lock = RW_LOCK_UNLOCKED;
- }
-
- return data;
-}
static inline void audit_data_free(struct audit_inode_data *data)
{
@@ -659,20 +672,58 @@ audit_receive_watch_exit:
int audit_inode_alloc(struct inode *inode)
{
- if (inode) {
- inode_audit_data(inode) = audit_data_alloc();
- if (!inode_audit_data(inode))
- return ENOMEM;
- }
+ struct audit_inode_data *data;
+ struct audit_inode_data **list;
+ int h = hash_ptr(inode, auditfs_hash_bits);
+ data = kmalloc(sizeof(struct audit_inode_data), GFP_KERNEL);
+ if (!data)
+ return -ENOMEM;
+
+ INIT_HLIST_HEAD(&data->watchlist);
+ data->wentry = NULL;
+ data->lock = RW_LOCK_UNLOCKED;
+ data->inode = inode;
+ data->next_hash = NULL;
+
+ /* Add it to the hash table */
+ list = &auditfs_hash_table[h];
+ spin_lock(&auditfs_hash_lock);
+
+ while (*list && (unsigned long)((*list)->inode) < (unsigned long)inode)
+ list = &(*list)->next_hash;
+
+ data->next_hash = *list;
+ *list = data;
+
+ spin_unlock(&auditfs_hash_lock);
return 0;
}
void audit_inode_free(struct inode *inode)
{
- if (inode)
- audit_data_free(inode_audit_data(inode));
+ struct audit_inode_data *data = NULL;
+ int h = hash_ptr(inode, auditfs_hash_bits);
+ struct audit_inode_data **list;
+
+ if (!inode)
+ return;
+
+ spin_lock(&auditfs_hash_lock);
+ list = &auditfs_hash_table[h];
+ while (*list && (unsigned long)((*list)->inode) < (unsigned long)inode)
+ list = &(*list)->next_hash;
+
+ if (*list && (*list)->inode == inode) {
+ data = *list;
+ *list = data->next_hash;
+ }
+ spin_unlock(&auditfs_hash_lock);
+
+ if (data)
+ audit_data_free(data);
}
+
/*
* When we delete a dentry we check to see whether or not we're being
* watched. If we are watched, we have to put back our reference to
@@ -712,6 +763,25 @@ int audit_filesystem_init(void)
if (!audit_wentry_cache)
goto audit_filesystem_init_fail;
+ /* Set up hash table for inode objects */
+ auditfs_hash_bits = long_log2(auditfs_cache_buckets);
+ if (auditfs_cache_buckets != (1 << auditfs_hash_bits)) {
+ auditfs_hash_bits++;
+ auditfs_cache_buckets = 1 << auditfs_hash_bits;
+ printk(KERN_NOTICE
+ "%s: auditfs_cache_buckets set to %d (bits %d)\n",
+ __FUNCTION__, auditfs_cache_buckets, auditfs_hash_bits);
+ }
+
+ auditfs_hash_table = kmalloc(auditfs_cache_buckets * sizeof(void *), GFP_KERNEL);
+
+ if (!auditfs_hash_table) {
+ printk(KERN_NOTICE "No memory to initialize auditfs cache.\n");
+ goto audit_filesystem_init_fail;
+ }
+
+ memset(auditfs_hash_table, 0, auditfs_cache_buckets * sizeof(void *));
+
ret = 0;
goto audit_filesystem_init_exit;
--
dwmw2
7 a.m.
On Wed, 2005-05-25 at 16:59 +0100, David Woodhouse wrote:
What I'm doing with it _is_ a bit dirty. Looks something like
this.
It compiles but I haven't tested it yet or even really done a second
read through the code
Seems to work OK if I also do this...
--- linux-2.6.9/kernel/audit.c~ 2005-05-26 11:25:59.000000000 +0100
+++ linux-2.6.9/kernel/audit.c 2005-05-26 12:25:41.000000000 +0100
@@ -544,7 +544,6 @@ int __init audit_init(void)
audit_sock->sk_sndtimeo = MAX_SCHEDULE_TIMEOUT;
skb_queue_head_init(&audit_skb_queue);
audit_initialized = 1;
- audit_filesystem_init();
audit_enabled = audit_default;
audit_log(NULL, AUDIT_KERNEL, "initialized");
return 0;
--- linux-2.6.9/fs/inode.c~ 2005-05-26 11:25:59.000000000 +0100
+++ linux-2.6.9/fs/inode.c 2005-05-26 12:26:29.000000000 +0100
@@ -1379,6 +1379,7 @@ void __init inode_init(unsigned long mem
inode_cachep = kmem_cache_create("inode_cache", sizeof(struct inode),
0, SLAB_PANIC, init_once, NULL);
set_shrinker(DEFAULT_SEEKS, shrink_icache_memory);
+ audit_filesystem_init();
}
void init_special_inode(struct inode *inode, umode_t mode, dev_t rdev)
--
dwmw2
6:55 a.m.
On Wed, 2005-05-25 at 15:11 +0100, David Woodhouse wrote:
As an added bonus, this should prevent it oopsing if a task
doesn't have
an audit context but touches a watched inode.
Hm, seems I lied about this bit. Although I thought it _really_ hard, my
emacs wasn't listening. I had to actually type it before it worked...
--- linux-2.6.9/kernel/auditfs.c~ 2005-05-26 11:25:59.000000000 +0100
+++ linux-2.6.9/kernel/auditfs.c 2005-05-26 12:41:48.000000000 +0100
@@ -804,7 +804,7 @@ int audit_notify_watch(struct inode *ino
if (likely(!audit_enabled))
return 0;
- if (!inode)
+ if (!inode || !current->audit_context)
return 0;
data = inode_audit_data(inode);
This means that tasks with an audit context will get their actions
logged when they touch a watched inode, but actions performed by tasks
_without_ an audit context will not by logged.
This corresponds to the use of 'possible' with audit rules, which
allocates a context but does not mark it as auditable. Only if something
else happens which marks the context as auditable, such as a filesystem
watch triggering, will the syscall in question get logged.
--
dwmw2
7:49 a.m.
On Thursday 26 May 2005 07:55, David Woodhouse wrote:
This means that tasks with an audit context will get their actions
logged when they touch a watched inode, but actions performed by tasks
_without_ an audit context will not by logged.
This seems like a source of unintended error. How do we let the system admin
know that they've lost events because they needed to specify possible? Could
an audit context be created on demand? The fact that it's a watched inode
would indicate that auditing is intended.
-Steve
8:07 a.m.
On Thu, 2005-05-26 at 08:49 -0400, Steve Grubb wrote:
This seems like a source of unintended error. How do we let the
system admin
know that they've lost events because they needed to specify possible?
The same way we let the sysadmin know that they've lost events because
they needed to specify a rule for whatever they wanted to watch -- i.e.
not at all, because it's not up to us to second-guess possible errors in
the configuration.
Could an audit context be created on demand?
Perhaps, but not nicely. And by that time we've already failed to log
the syscall information at syscall_audit_entry() anyway.
We could perhaps create one and record partial information, but I'm not
sure I see the point. If the admin said that this task wasn't to be
audited, why would we disobey?
The fact that it's a watched inode would indicate that auditing
is
intended.
Conversely, the fact that it's a task for which no auditing was
specified would indicate that auditing is not intended.
--
dwmw2
8:20 a.m.
On Thursday 26 May 2005 09:07, David Woodhouse wrote:
We could perhaps create one and record partial information, but
I'm not
sure I see the point.
We are talking about the case where an audit event would be generated but
can't because there's no context? If so, we have to record the event somehow.
-Steve
8:38 a.m.
On Thu, 2005-05-26 at 09:20 -0400, Steve Grubb wrote:
We are talking about the case where an audit event would be generated
but
can't because there's no context? If so, we have to record the event somehow.
We're talking about the case where a watched file has been touched, but
the sysadmin has explicitly told the kernel that this task is _not_ to
be audited.
--
dwmw2
7149
days inactive
7150
days old
linux-audit@lists.linux-audit.osci.io
12 comments
3 participants
participants (3)
-
David Woodhouse -
Steve Grubb -
Timothy R. Chavez