2:30 p.m.
I've pulled in the patch which fixes the module signing code from what
will be the RHEL U2 kernel. This ought to boot reliably on PPC64 now.
It would be very useful to get the refcounting of inode audit data tested.
* Thu Jun 2 2005 David Woodhouse <dwmw2(a)redhat.com> audit.53
- Fix module signing oops
- Sparse allocation of inode audit data
* Thu Jun 2 2005 David Woodhouse <dwmw2(a)redhat.com> audit.52
- Fix thinko in sys_socketcall. Having painstakingly copied the arguments
in from userspace, let's then _use_ that copy instead of the users' copy.
- Abolish struct audit_wentry by merging it with struct audit_watch.
- Fix up error handling in fs/namei.c
--
dwmw2
3:15 p.m.
On Thursday 02 June 2005 15:30, David Woodhouse wrote:
It would be very useful to get the refcounting of inode audit data
tested.
I tested this. I started my test of inserting rules. Stopped it. Deleted all
rules, and rebooted. On the way down it did some interesting things. The
rules came back to life and started scrolling across the screen and I had 2
or 3 oopses. This was after syslog had stopped logging so I couldn't get
anything.
It seems to have hammered init also since I wind up at a point that is neither
alive or dead. I can type and keys are echoed to screen and audit stuff
occasionally appears, but nothing else happens.
-Steve
4:46 p.m.
On Thursday 02 June 2005 16:15, Steve Grubb wrote:
This was after syslog had stopped logging so I couldn't get
anything.
I have some more information. There are 2 problems. One is that the rules come
back to life. If you set a watch on /bin/sh, you see when shell scripts get
called on shutdown.
The other problem is an oops. It only occurs if auditing is enabled during
shutdown. When auditing is disabled, you get the inodes busy message. I also
found that I get the oops on shutdown with only this command entered:
auditctl -w /var/gdm/.gdmfifo -k pipe -p rwea
The information I was able to collect is:
EIP: 0xf0888105
EIP is at ext3_put_super
process unmount (pid 3380
call trace:
generic_shutdown_super
kill_block_super
deactivate_super
sys_unmount
audit_syscall_entry
do_syscall_trace
syscall_call
/etc/rc0.d/S01halt: line 14: 3380 segmentation fault
unmounting filesystems
umount2 invalid argument
Checking gdb for EIP shows an invalid address.
-Steve
4:53 p.m.
On Thu, 2005-06-02 at 17:46 -0400, Steve Grubb wrote:
The information I was able to collect is:
EIP: 0xf0888105
EIP is at ext3_put_super
process unmount (pid 3380
That really isn't too useful. Can you hook up a serial console? I did
ask you that on IRC but then noticed you'd gone, so I assumed you were
gone for the day.
--
dwmw2
6:04 a.m.
On Thu, 2005-06-02 at 17:46 -0400, Steve Grubb wrote:
The information I was able to collect is:
EIP: 0xf0888105
EIP is at ext3_put_super
process unmount (pid 3380
I've now reproduced this with a serial console attached. This part of
the output would have been more useful:
Assertion failure in ext3_put_super() at fs/ext3/super.c:425:
"list_empty(&sbi->s_orphan)"
------------[ cut here ]------------
kernel BUG at fs/ext3/super.c:425!
Checking gdb for EIP shows an invalid address.
You don't really need to look up the line number for a BUG(), because it
tells you. But for reference: that EIP was in a module, so you have to
look at that module for debugging information instead of vmlinux, and
use the address relative to the (unrelocated) function rather than the
absolute address.
(gdb) list *ext3_put_super+0xf2
0x9c1a is in ext3_put_super (fs/ext3/super.c:425).
420 * isn't empty. The on-disk one can be non-empty if we've
421 * detected an error and taken the fs readonly, but the
422 * in-memory list had better be clean by this point. */
423 if (!list_empty(&sbi->s_orphan))
424 dump_orphan_list(sb, sbi);
425 J_ASSERT(list_empty(&sbi->s_orphan));
426
427 invalidate_bdev(sb->s_bdev, 0);
428 if (sbi->journal_bdev && sbi->journal_bdev != sb->s_bdev)
{
429 /*
--
dwmw2
8:29 a.m.
On Thu, 2005-06-02 at 17:46 -0400, Steve Grubb wrote:
The other problem is an oops. It only occurs if auditing is enabled
during shutdown. When auditing is disabled, you get the inodes busy
message.
They're both manifestations of the same problem -- I don't think we were
unpinning dentries at all on unmount. I think we can ditch the dentry
pinning scheme, and rely on the implicit pinning which we get from using
a bit in inode->i_state to note the presence of audit data in the hash
table for the inode.
This will mean that prune_icache() will leave the inode alone, because
it merely checks whether i_state is zero or not.
I'm playing with something like this... note the conditional debugging
in audit_inode_free(), because it's too noisy if it does it for all file
systems. You might want to change the block device it prints messages
for, if you aren't testing on /dev/hda6 as I am.
--- kernel-2.6.9-running//linux-2.6.9/fs/inode.c 2005-06-03 14:14:18.000000000 +0100
+++ kernel-2.6.9/linux-2.6.9/fs/inode.c 2005-06-03 11:50:24.000000000 +0100
@@ -262,7 +262,7 @@ void clear_inode(struct inode *inode)
bd_forget(inode);
if (inode->i_cdev)
cd_forget(inode);
- inode->i_state = I_CLEAR | (inode->i_state & I_AUDIT);
+ inode->i_state = I_CLEAR;
}
EXPORT_SYMBOL(clear_inode);
--- kernel-2.6.9-running//linux-2.6.9/include/linux/fs.h 2005-06-03 12:36:12.000000000
+0100
+++ kernel-2.6.9/linux-2.6.9/include/linux/fs.h 2005-06-03 11:50:24.000000000 +0100
@@ -986,7 +986,6 @@ struct super_operations {
#define I_FREEING 16
#define I_CLEAR 32
#define I_NEW 64
-#define I_AUDIT 128
#define I_DIRTY (I_DIRTY_SYNC | I_DIRTY_DATASYNC | I_DIRTY_PAGES)
--- kernel-2.6.9-running//linux-2.6.9/kernel/auditfs.c 2005-06-03 14:03:50.000000000
+0100
+++ kernel-2.6.9/linux-2.6.9/kernel/auditfs.c 2005-06-03 11:50:24.000000000 +0100
@@ -36,7 +36,7 @@
#include <linux/module.h>
#include <asm/uaccess.h>
-#if 0
+#if 1
#define dprintk(...) do { } while(0)
#define __print_symbol(x, y) do { } while(0)
#else
@@ -58,7 +58,7 @@ struct audit_skb_list {
void *memblk;
size_t size;
};
-extern spinlock_t inode_lock;
+
static int audit_nr_watches;
static int audit_pool_size;
@@ -106,9 +106,7 @@ static struct audit_inode_data *audit_da
list = &auditfs_hash_table[h];
spin_lock(&auditfs_hash_lock);
- /* I_AUDIT bit can only be changed under auditfs_hash_lock, so no need
- to lock inode_lock (on all known hardware) */
-
+
while (*list && (unsigned long)((*list)->inode) < (unsigned long)inode) {
dprintk("list %p -> %p\n", list, *list);
list = &(*list)->next_hash;
@@ -116,12 +114,6 @@ static struct audit_inode_data *audit_da
if (*list && (*list)->inode == inode)
ret = *list;
- if (ret && !(inode->i_state & I_AUDIT)) {
- printk("Eep. Inode #%lu has data %p but not I_AUDIT\n",
- inode->i_ino, ret);
- } else if (!ret && (inode->i_state & I_AUDIT)) {
- printk("Eep. Inode #%lu has I_AUDIT but not data\n", inode->i_ino);
- }
if (ret) {
ret->count++;
} else if (allocate) {
@@ -134,9 +126,6 @@ static struct audit_inode_data *audit_da
ret->watch = NULL;
ret->lock = RW_LOCK_UNLOCKED;
ret->inode = inode;
- spin_lock(&inode_lock);
- inode->i_state |= I_AUDIT;
- spin_unlock(&inode_lock);
ret->next_hash = *list;
ret->count = 2;
*list = ret;
@@ -146,7 +135,6 @@ static struct audit_inode_data *audit_da
ret, ret->inode, ret->inode->i_ino, ret->count,
__builtin_return_address(0));
__print_symbol("%s\n", __builtin_return_address(0));
}
- out:
spin_unlock(&auditfs_hash_lock);
return ret;
@@ -157,26 +145,18 @@ static struct audit_inode_data *audit_da
/* Unpin the dentry stored at watch->w_dentry. */
static inline void audit_unpin(struct audit_watch *watch)
{
-#if 0
if (watch && watch->w_dentry) {
- printk("unpin dentry %s\n", watch->w_dentry->d_name.name);
- BUG_ON(atomic_read(&watch->w_dentry->d_count) == 1);
dput(watch->w_dentry);
watch->w_dentry = NULL;
}
-#endif
}
/* Pin the dentry and store it at watch->w_dentry. */
static inline void audit_pin(struct audit_watch *watch,
struct dentry *dentry)
{
-#if 0
- if (watch && !watch->w_dentry) {
+ if (watch && !watch->w_dentry)
watch->w_dentry = dget(dentry);
- printk("pin dentry %s\n", dentry->d_name.name);
- }
-#endif
}
static inline struct audit_watch *audit_watch_fetch(const char *name,
@@ -414,24 +394,6 @@ static inline void audit_drain_watchlist
}
}
-static void audit_data_unhash(struct audit_inode_data *data)
-{
- int h = hash_ptr(data->inode, auditfs_hash_bits);
- struct audit_inode_data **list = &auditfs_hash_table[h];
-
- while (*list && (unsigned long)((*list)->inode) < (unsigned
long)data->inode)
- list = &(*list)->next_hash;
-
- BUG_ON(*list != data);
- *list = data->next_hash;
-
- spin_lock(&inode_lock);
- data->inode->i_state &= ~I_AUDIT;
- spin_unlock(&inode_lock);
- data->inode = NULL;
-}
-
-
static void audit_data_put(struct audit_inode_data *data)
{
if (!data)
@@ -452,13 +414,22 @@ static void audit_data_put(struct audit_
if (!data->count) {
/* We are last user. Remove it from the hash table to
disassociate it from its inode */
- if (data->inode)
- audit_data_unhash(data);
+ int h = hash_ptr(data->inode, auditfs_hash_bits);
+
+ struct audit_inode_data **list = &auditfs_hash_table[h];
+
+ while (*list && (unsigned long)((*list)->inode) < (unsigned
long)data->inode)
+ list = &(*list)->next_hash;
+
+ BUG_ON(*list != data);
+ *list = data->next_hash;
+ data->inode = NULL;
+
spin_unlock(&auditfs_hash_lock);
audit_drain_watchlist(data);
-
spin_lock(&auditfs_hash_lock);
+
/* Check whether to free it or return it to the pool */
if (audit_nr_watches > audit_pool_size) {
dprintk("Back to pool. %d watches, %d in pool\n", audit_nr_watches,
audit_pool_size);
@@ -470,6 +441,7 @@ static void audit_data_put(struct audit_
kfree(data);
}
}
+
spin_unlock(&auditfs_hash_lock);
}
@@ -564,8 +536,7 @@ struct audit_watch *audit_watch_get(stru
{
if (watch) {
BUG_ON(!atomic_read(&watch->w_count));
- printk("Increase count on watch %p to %d\n",
- watch, atomic_inc_return(&watch->w_count));
+ atomic_inc(&watch->w_count);
}
return watch;
@@ -573,15 +544,8 @@ struct audit_watch *audit_watch_get(stru
void audit_watch_put(struct audit_watch *watch)
{
- int new;
-
- if (watch) {
- new = atomic_dec_return(&watch->w_count);
- if (!new)
- audit_watch_free(watch);
- printk("Reduce count on watch %p to %d\n",
- watch, new);
- }
+ if (watch && atomic_dec_and_test(&watch->w_count))
+ audit_watch_free(watch);
}
/*
@@ -806,14 +770,7 @@ void audit_inode_free(struct inode *inod
{
struct audit_inode_data *data = audit_data_get(inode, 0);
- if (inode->i_sb->s_dev == MKDEV(3,6))
- printk("Put inode #%lu, data %p\n", inode->i_ino, data);
-
if (data) {
- spin_lock(&auditfs_hash_lock);
- audit_data_unhash(data);
- spin_unlock(&auditfs_hash_lock);
-
audit_drain_watchlist(data);
audit_watch_put(data->watch);
data->watch = NULL;
@@ -830,7 +787,6 @@ void audit_inode_free(struct inode *inod
*/
void audit_dentry_unpin(struct dentry *dentry)
{
-#if 0
struct audit_inode_data *parent;
struct audit_watch *watch;
@@ -849,7 +805,6 @@ void audit_dentry_unpin(struct dentry *d
audit_watch_put(watch);
}
audit_data_put(parent);
-#endif
}
int audit_filesystem_init(void)
--
dwmw2
7245
days inactive
7246
days old
linux-audit@lists.linux-audit.osci.io
5 comments
2 participants
participants (2)
-
David Woodhouse -
Steve Grubb