untrusted string optimization
by Steve Grubb
Hello,
I was looking at some ascii charts trying to interpret an untrusted string and
realized something. We have this test in audit_log_untrustedstring() in
audit.c:
if (*p == '"' || *p == ' ' || *p < 0x20 || *p > 0x7f) {
It turns out that space is hex 20. So we could have:
if (*p == '"' || *p < 0x21 || *p > 0x7f) {
and have one less compare in that loop.
-Steve
19 years, 7 months
[PATCH] Untrusted string logging
by Steve Grubb
Hello,
As I was working on the new parsers for ausearch, I ran across some issues
with untrusted string logging. Attached is a patch to fix this. Let me go
over them as they appear in the patch.
* In audit_log_vformat, we expand the skb just enough memory to cover current
needs. There's no hysteresis. Imagine a call to log untrusted string passing
a string of 4024 bytes and has a space in it. We will make 3000 calls to
reallocate the buffer since audit_log_hex calls audit_log_vformat for every
character it needs to convert. This is fixed by allocating in multiples of
AUDIT_BUFSIZ.
* If vsnprintf returns -1, it will mess up the sk buffer space accounting.
This is fixed by not calling skb_put with bogus len values.
* audit_log_hex was a loop that called audit_log_vformat with %02X for each
character. This is very inefficient since conversion from unsigned character
to Ascii representation is essentially masking, shifting, and byte lookups.
Also, the length of the converted string is well known - its twice the
original. Fixed by rewriting the function.
*audit_log_untrustedstring had no comments. This makes it hard for someone to
understand what the string format will be.
* audit_log_d_path was never fixed to use untrustedstring. This could mess up
user space parsers. This was fixed to make a temp buffer, call d_path, and
log temp buffer using untrustedstring.
*avc messages print the comm string without escaping. This was not fixed when
we introduced untrustedstring and modified auditsc.c.
-Steve Grubb
19 years, 7 months
[PATCH 0/3] Buffer audit messages directly to skb
by Chris Wright
The following series writes audit messages directly to skb rather
than formatting to a tmp buffer, and copying to some number of skbs
(potentially fragmenting the message across skbs). This is done in
three steps.
1) audit-buffer-alloc.patch - adds simpler alloc/free helper functions
2) audit-buffer-expand.patch - moves to a dynamic tmp buffer which can
be expanded as needed. copy message to a single skb.
3) audit-buffer-skb.patch - eliminate tmp buffer and write directly to skb.
kernel/audit.c | 232 ++++++++++++++++++++++++++++-----------------------------
1 files changed, 118 insertions(+), 114 deletions(-)
thanks,
-chris
19 years, 7 months
ausearch documentation
by Daniel H. Jones
While playing around with a test case for ausearch, I found the man page
could have been more helpful. The message types obviously need to be
updated. But in addition to that, it's not at all clear what options can
be combined. Not all options will apply to all message types. For the
purposes of an evaluation, the man page is the functional specification.
It needs to be unambiguous.
--
Thanks,
Dan Jones
IBM Linux Technology Center, Security
512-838-1794 (T/L 678-1794)
hotrats(a)us.ibm.com
19 years, 7 months
audit 0.8.1 released
by Steve Grubb
Hello,
I've just released a new version of the audit daemon. It can be downloaded
from http://people.redhat.com/sgrubb/audit It will also be in rawhide
tomorrow. The Changelog is:
- Fix code to "or" uid & gid checks for ausearch -ua & -ga
- Change msg() to audit_msg() to avoid conflicts
- Parse socket messages for hostname in ausearch
This release fixes bugs in ausearch and adds socket address support. A library
function was renamed since it clashes with a function name in ipop3d. I guess
people are starting to write programs with audit support. :)
Please let me know if there are any problems.
-Steve
19 years, 7 months
[PATCH] move audit_dentry_unpin
by Timothy R. Chavez
Hello,
This patch fixes the error that Loulwa reported was hanging her system. What
was happening was that audit_dentry_unpin was being called under the
protection of the same spinlocks it was trying to hold itself (via the dput()
it was calling). To fix this, I moved audit_dentry_unpin outside of the
spinlocks where it belonged in the first place.
-tim
--- fs/dcache.c 2005-05-17 13:59:32.968013816 -0500
+++ ../../kernel-2.6.9~working/linux-2.6.9/fs/dcache.c 2005-05-17
13:43:09.000000000 -0500
@@ -1109,13 +1109,15 @@ out:
void d_delete(struct dentry * dentry)
{
+ audit_dentry_unpin(dentry);
+
/*
* Are we the only user?
*/
+
spin_lock(&dcache_lock);
spin_lock(&dentry->d_lock);
- audit_dentry_unpin(dentry);
if (atomic_read(&dentry->d_count) == 1) {
dentry_iput(dentry);
-tim
19 years, 7 months
audit.36 kernel
by David Woodhouse
Contains Tim's #7U5' patch, Steve's patches for message types and
cleaing up untrusted string logging.
Still doesn't have error handling for the netlink_unicast() call in
audit_send_reply(). Every time I look at that I just cannot bring myself
to add the 'skb_get(); /* netlink frees */' which prefixes the
netlink_unicast() elsewhere. That just looks _wrong_ to me -- there has
to be something wrong with the refcounting.
--
dwmw2
19 years, 7 months
Opinion please
by Timothy R. Chavez
Hello,
Serge pointed out something to me on an internal list here and I wanted to
poll for opinions. Which would you prefer? I think adding another parameter
to audit_send_reply to specify GFP_ATOMIC/KERNEL would be simplest. But is
it wise to do so?
Here's what he had to say:
Hey Tim,
it looks like w_master is now adequately protected by the spinlock.
Only problem is that you might sleep under rcu_read_lock(). In
audit_list_watches(), you call audit_send_watch() from under
rcu_read_lock. audit_send_watch() calls audit_to_transport, which calls
kmalloc(GFP_KERNEL). Switch that to GFP_ATOMIC.
audit_send_watch() also calls audit_send_reply(), which calls
alloc_skb(GFP_KERNEL), and netlink_unicast. You could patch
audit_send_reply() to take a gfp_mask argument, and send
gfp_mask=GFP_ATOMIC from audit_send_watch().
But I'm not sure whether netlink_unicast is a problem (sure looks like
it - it puts current on a waitqueue for one thing) or what you should do
about it. You might just need to build a list of things to send under
the rcu_read_lock(), then send each element once you rcu_read_unlock().
So something like:
struct audit_skb_list {
struct hlist_node list;
void *memblk;
size_t size;
};
audit_list_watches(int pid, int seq)
{
struct hlist_head skb_list;
struct audit_skb_list listentry;
struct hlist_node *tmp, *pos;
INIT_HLIST_HEAD(&skb_list);
rcu_read_lock();
hlist_for_each_entry_rcu(...) {
tmp = audit_build_watch(pid, seq, wentry->w_watch);
if (tmp)
hlist_add_head(tmp, &skb_list);
}
rcu_read_unlock();
hlist_for_each_entry_safe(listentry, pos, tmp, &skb_list, list) {
audit_send_reply(pid, seq, AUDIT_WATCH_LIST, 0, 1,
listentry->memblk, listentry->size);
kfree(listentry);
}
audit_send_reply(pid, seq, AUDIT_WATCH_LIST, 1, 1, NULL, 0);
}
--
-tim
19 years, 7 months
2 more message types for pam
by Steve Grubb
Hello,
I was working on the pam library today and found that I needed 2 more types
from the kernel to properly support pam. The attached patch adds these types.
-Steve
19 years, 7 months
