Re: Refactoring src/ausearch-report.c:output_interpreted_node()
Thanks Steve,
Patch to follow this weekend.
Rgds
On Wed, 2014-10-01 at 18:28 -0400, Steve Grubb wrote:
On Thursday, October 02, 2014 07:52:47 AM Burn Alting wrote:
On Wed, 2014-10-01 at 17:19 -0400, Steve Grubb wrote:
On Thursday, October 02, 2014 07:08:13 AM Burn Alting wrote:
On Wed, 2014-10-01 at 14:54 -0400, Steve Grubb wrote:
I am uncertain what effect of accepting this additional format would
have when adding rules to the running audit system - i.e.
audit_name_to_msg_type() is called by autrace/auditctl when parsing
rules (ie the msgtype field name).
I think ausearch-report.c might be the place that needs updating.
So, could we modify output_interpreted_node() to no longer re-parse the
[node=node] type=type msg=audit(epochsecs.msecs:serial)
header and pass both the lnode and llist-e which has this data already
as the code
if (num == -1) {
// see if we are older and wiser now.
bptr = strchr(str, '[');
if (bptr bptr ptr) {
char *eptr;
bptr++;
eptr = strchr(bptr, ']');
if (eptr) {
*eptr = 0;
errno = 0;
num = strtoul(bptr, NULL, 10);
*eptr = ']';
if (errno)
num = -1;
}
}
}
which parses for
type=.*[n].*
is no longer needed as we don't have that format any more?
That is a very loose check for UNKNOWN[]. If you see a performance
improvement by refactoring this function, please send a patch. The output
needs to be identical to the old way.
Thanks,
-Steve
I can provide a patch to refactor this part of the code, but I want to
confirm there is no longer a need to parse for
type=some_text '[' integer_type ']' some_other_text
While this may have been implied by the code, the fact is that [ ] would only
be in type fields when its unknown[].
given my refactoring will rely upon the parsing already done by
lib/lookup_table.c:audit_name_to_msg_type(). Remember this routine only
parses for
Given
type=type_value
then
type_value
is parsed for
- a known string
- a long integer number, n, found in the specific string
UNKNOWN[n]
- a long integer number, n, found in the specific string
n
These 3 formats are all that it can ever be. So, I think you have a correct
understanding.
-Steve
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
Re: Audit format utility
On Thursday, October 02, 2014 07:44:32 AM Burn Alting wrote: On Wed, 2014-10-01 at 14:44 -0400, Steve Grubb wrote: Further, it has a couple of immediate issues given it's using libauparse. - it is lossy in that it wont parse poorly formed audit events (see the op key value pair below) [burn@swtf auformat]$ cat add_user.txt node=swtf.swtf.dyndns.org type=ADD_USER msg=audit(1411871714.393:47872): user pid=13455 uid=0 auid=500 ses=11 subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 msg='op=adding home directory id=502 exe=/usr/sbin/useradd hostname=? addr=? terminal=pts/2 res=success' [burn@swtf auformat]$ ./auformat %node %date %time %milli % serial: type=%TYPE msg=%msg op=%op auid=%auid pid=%pid path=% path exe=%exe subj=%subj hostname=%hostname terminal=%terminal res=%res\n add_user.txt swtf.swtf.dyndns.org 09/28/2014 12:35:14 393 47872: type=ADD_USER msg= op=adding auid=500 pid=13455 path= exe=/usr/sbin/useradd subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 hostname=? terminal=pts/2 res=success [burn@swtf auformat]$ We loose the strings - 'user' before the pid key Which is meaningless in this case. - op='adding home directory' becomes op'adding' This is particularly important for incorrectly formatted application level audit sent via auditd. This is a problem in the shadow-utils package. It is the one that I'm currently having to re-do for this reason and many more. Upstream seems to have taken a stab at re-doming the audit events and pretty much used it like syslog. I suppose my concern is that until we have fixed all the incorrectly formatted key values, auparse is going to loose information. OK. I see. I really think we should get some tool created that can help identify these. It could be as simple as pushing into auparse, iterating across fields to to recreate the record, then diff the original and the recreated. From that, we can get fixes in place. I think shadow-utils is the package most affected. - 'rewinding' the event's cursor for each possible key, the call to auparse_first_record() in print_item(), is probably not what one would want - but then again, auformat is just a mock up at the moment. Well, if you want your fields in a specific order and its not the order in the event, then we have no choice. Note that the event is alrady parsed at this point so we are just literally changing the position in a linked list. The cost is a series of strcmp calls. - one looses the parsing 'fix-up' that ausearch does in src/ausearch-report.c:output_interpreted_node() Not sure what fix-up we are talking about. The intention is that auparse completely mimicks ausearch's interpretation ability (which ausearch was switched over to use auparse a few releases back). By 'fix-up' I meant the code like // Some user messages have msg='uid=500 in this case // skip the msg= piece since the real stuff is the uid= ... // Value side has commas and another field exists // Known: LABEL_LEVEL_CHANGE banners=none,none // Known: ROLL_ASSIGN new-role=r,r // Known: any MAC LABEL can potentially have commas etc Auparse should handle these. - to build a complete event, having addressed the 'rewinding' issue, would make the format look very messy - you would need to include every possible key to print all key/values. If you wanted that, yeah. But I am thinking of cases where one may not want every field. For example, you might do something like this to check file access: # ausearch --start today -m path --raw | auformat 'auid=%AUID res=%SUCCESS name=%NAME\n' - one should add event separation so that further tools could process the data more easily. I am thinking of 1 event per line. This is kind of a requirement of Map Reduce. So you expect the complete event of my tailing audit.log node=swtf.swtf.dyndns.org type=SYSCALL msg=audit(1412198543.190:141570): arch=c03e syscall=59 success=yes exit=0 a0=1a2d530 a1=1a2d350 a2=1a06f10 a3=20 items=2 ppid=19529 pid=32647 auid=500 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=pts0 ses=1 comm=tail exe=/usr/bin/tail subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=cmds node=swtf.swtf.dyndns.org type=EXECVE msg=audit(1412198543.190:141570): argc=3 a0=tail a1=-f a2=/var/log/audit/audit.log node=swtf.swtf.dyndns.org type=CWD msg=audit(1412198543.190:141570): cwd=/home/burn node=swtf.swtf.dyndns.org type=PATH
[PATCH 0/7] audit fsnotify cleanups for watches and trees
This is a collection of patches to clean up some issues discovered while implementing audit by exe path. They compile and have been lightly tested. I'd be interested in feedback about approaches or details or grossly misunderstanding some fundamental concepts. Thanks! Richard Guy Briggs (7): audit: put rule existence check in canonical order audit: cull redundancy in audit_rule_change audit: eliminate string copy for new tree rules audit: optimize add to parent skipping needless search and consuming parent ref audit: remove redundant watch refcount audit: remove extra audit_get_parent() audit: rename audit_log_remove_rule to disambiguate for trees kernel/audit_tree.c | 13 +++-- kernel/audit_watch.c | 29 - kernel/auditfilter.c | 34 +++--- 3 files changed, 34 insertions(+), 42 deletions(-) -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH 4/7] audit: optimize add to parent skipping needless search and consuming parent ref
When parent has just been created there is no need to search for the parent in
the list. Add a parameter to skip the search and consume the parent reference
no matter what happens.
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
kernel/audit_watch.c | 23 +++
1 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index ad9c168..f209448 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -372,15 +372,20 @@ static int audit_get_nd(struct audit_watch *watch, struct
path *parent)
}
/* Associate the given rule with an existing parent.
- * Caller must hold audit_filter_mutex. */
+ * Caller must hold audit_filter_mutex.
+ * Consumes parent reference. */
static void audit_add_to_parent(struct audit_krule *krule,
- struct audit_parent *parent)
+ struct audit_parent *parent,
+ int new)
{
struct audit_watch *w, *watch = krule-watch;
int watch_found = 0;
BUG_ON(!mutex_is_locked(audit_filter_mutex));
+ if (new)
+ goto not_found;
+
list_for_each_entry(w, parent-watches, wlist) {
if (strcmp(watch-path, w-path))
continue;
@@ -396,12 +401,15 @@ static void audit_add_to_parent(struct audit_krule *krule,
break;
}
+not_found:
if (!watch_found) {
- audit_get_parent(parent);
watch-parent = parent;
list_add(watch-wlist, parent-watches);
- }
+ } else
+ /* match get in audit_find_parent or audit_init_parent */
+ audit_put_parent(parent);
+
list_add(krule-rlist, watch-rules);
}
@@ -413,6 +421,7 @@ int audit_add_watch(struct audit_krule *krule, struct
list_head **list)
struct audit_parent *parent;
struct path parent_path;
int h, ret = 0;
+ int new = 0;
mutex_unlock(audit_filter_mutex);
@@ -433,12 +442,10 @@ int audit_add_watch(struct audit_krule *krule, struct
list_head **list)
ret = PTR_ERR(parent);
goto error;
}
+ new = 1;
}
- audit_add_to_parent(krule, parent);
-
- /* match get in audit_find_parent or audit_init_parent */
- audit_put_parent(parent);
+ audit_add_to_parent(krule, parent, new);
h = audit_hash_ino((u32)watch-ino);
*list = audit_inode_hash[h];
--
1.7.1
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH 5/7] audit: remove redundant watch refcount
Remove extra layer of audit_{get,put}_watch() calls.
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
kernel/audit_watch.c |5 +
kernel/auditfilter.c |7 ---
2 files changed, 1 insertions(+), 11 deletions(-)
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index f209448..c707afb 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -203,7 +203,6 @@ int audit_to_watch(struct audit_krule *krule, char *path,
int len, u32 op)
if (IS_ERR(watch))
return PTR_ERR(watch);
- audit_get_watch(watch);
krule-watch = watch;
return 0;
@@ -306,7 +305,6 @@ static void audit_update_watch(struct audit_parent *parent,
* new watch.
*/
audit_put_watch(nentry-rule.watch);
- audit_get_watch(nwatch);
nentry-rule.watch = nwatch;
list_add(nentry-rule.rlist, nwatch-rules);
list_add_rcu(nentry-list,
audit_inode_hash[h]);
@@ -392,8 +390,7 @@ static void audit_add_to_parent(struct audit_krule *krule,
watch_found = 1;
- /* put krule's and initial refs to temporary watch */
- audit_put_watch(watch);
+ /* put krule's ref to temporary watch */
audit_put_watch(watch);
audit_get_watch(w);
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index facd704..5675916 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -563,8 +563,6 @@ exit_nofree:
return entry;
exit_free:
- if (entry-rule.watch)
- audit_put_watch(entry-rule.watch); /* matches initial get */
if (entry-rule.tree)
audit_put_tree(entry-rule.tree); /* that's the temporary one */
audit_free_rule(entry);
@@ -942,8 +940,6 @@ static inline int audit_add_rule(struct audit_entry *entry)
return 0;
error:
- if (watch)
- audit_put_watch(watch); /* tmp watch, matches initial get */
return err;
}
@@ -951,7 +947,6 @@ error:
static inline int audit_del_rule(struct audit_entry *entry)
{
struct audit_entry *e;
- struct audit_watch *watch = entry-rule.watch;
struct audit_tree *tree = entry-rule.tree;
struct list_head *list;
int ret = 0;
@@ -992,8 +987,6 @@ static inline int audit_del_rule(struct audit_entry *entry)
mutex_unlock(audit_filter_mutex);
out:
- if (watch)
- audit_put_watch(watch); /* match initial get */
if (tree)
audit_put_tree(tree); /* that's the temporary one */
--
1.7.1
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH 7/7] audit: rename audit_log_remove_rule to disambiguate for trees
Rename audit_log_remove_rule() to audit_tree_log_remove_rule() to avoid
confusion with watch and mark rule removal/changes.
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
kernel/audit_tree.c |4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index ace72ed..866d403 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -450,7 +450,7 @@ static int tag_chunk(struct inode *inode, struct audit_tree
*tree)
return 0;
}
-static void audit_log_remove_rule(struct audit_krule *rule)
+static void audit_tree_log_remove_rule(struct audit_krule *rule)
{
struct audit_buffer *ab;
@@ -477,7 +477,7 @@ static void kill_rules(struct audit_tree *tree)
list_del_init(rule-rlist);
if (rule-tree) {
/* not a half-baked one */
- audit_log_remove_rule(rule);
+ audit_tree_log_remove_rule(rule);
rule-tree = NULL;
list_del_rcu(entry-list);
list_del(entry-rule.list);
--
1.7.1
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH 6/7] audit: remove extra audit_get_parent()
There appears to be an extra parent reference taken. Remove it.
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
kernel/audit_watch.c |1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index c707afb..eb53bc7 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -462,7 +462,6 @@ void audit_remove_watch_rule(struct audit_krule *krule)
audit_remove_watch(watch);
if (list_empty(parent-watches)) {
- audit_get_parent(parent);
fsnotify_destroy_mark(parent-mark, audit_watch_group);
audit_put_parent(parent);
}
--
1.7.1
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH 2/7] audit: cull redundancy in audit_rule_change
Re-factor audit_rule_change() to reduce the amount of code redundancy and
simplify the logic.
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
kernel/auditfilter.c | 20 +++-
1 files changed, 7 insertions(+), 13 deletions(-)
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 4a11697..e3378a4 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1064,30 +1064,24 @@ int audit_rule_change(int type, __u32 portid, int seq,
void *data,
int err = 0;
struct audit_entry *entry;
+ entry = audit_data_to_entry(data, datasz);
+ if (IS_ERR(entry))
+ return PTR_ERR(entry);
+
switch (type) {
case AUDIT_ADD_RULE:
- entry = audit_data_to_entry(data, datasz);
- if (IS_ERR(entry))
- return PTR_ERR(entry);
-
err = audit_add_rule(entry);
audit_log_rule_change(add_rule, entry-rule, !err);
- if (err)
- audit_free_rule(entry);
break;
case AUDIT_DEL_RULE:
- entry = audit_data_to_entry(data, datasz);
- if (IS_ERR(entry))
- return PTR_ERR(entry);
-
err = audit_del_rule(entry);
audit_log_rule_change(remove_rule, entry-rule, !err);
- audit_free_rule(entry);
break;
- default:
- return -EINVAL;
}
+ if (err || type == AUDIT_DEL_RULE)
+ audit_free_rule(entry);
+
return err;
}
--
1.7.1
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH 1/7] audit: put rule existence check in canonical order
Use same rule existence check order as audit_make_tree(), audit_to_watch(),
update_lsm_rule() for legibility.
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
kernel/auditfilter.c |2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index 40ed981..4a11697 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -163,7 +163,7 @@ static inline int audit_to_inode(struct audit_krule *krule,
struct audit_field *f)
{
if (krule-listnr != AUDIT_FILTER_EXIT ||
- krule-watch || krule-inode_f || krule-tree ||
+ krule-inode_f || krule-watch || krule-tree ||
(f-op != Audit_equal f-op != Audit_not_equal))
return -EINVAL;
--
1.7.1
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH 3/7] audit: eliminate string copy for new tree rules
New tree rules copy the path twice and discard the intermediary copy.
This saves one pointer at the expense of one path string copy.
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
kernel/audit_tree.c |9 +
kernel/auditfilter.c |5 +++--
2 files changed, 8 insertions(+), 6 deletions(-)
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index bd418c4..ace72ed 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -17,7 +17,7 @@ struct audit_tree {
struct list_head list;
struct list_head same_root;
struct rcu_head head;
- char pathname[];
+ char *pathname;
};
struct audit_chunk {
@@ -70,11 +70,11 @@ static LIST_HEAD(prune_list);
static struct fsnotify_group *audit_tree_group;
-static struct audit_tree *alloc_tree(const char *s)
+static struct audit_tree *alloc_tree(char *s)
{
struct audit_tree *tree;
- tree = kmalloc(sizeof(struct audit_tree) + strlen(s) + 1, GFP_KERNEL);
+ tree = kmalloc(sizeof(struct audit_tree), GFP_KERNEL);
if (tree) {
atomic_set(tree-count, 1);
tree-goner = 0;
@@ -83,7 +83,7 @@ static struct audit_tree *alloc_tree(const char *s)
INIT_LIST_HEAD(tree-list);
INIT_LIST_HEAD(tree-same_root);
tree-root = NULL;
- strcpy(tree-pathname, s);
+ tree-pathname = s;
}
return tree;
}
@@ -96,6 +96,7 @@ static inline void get_tree(struct audit_tree *tree)
static inline void put_tree(struct audit_tree *tree)
{
if (atomic_dec_and_test(tree-count))
+ kfree(tree-pathname);
kfree_rcu(tree, head);
}
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index e3378a4..facd704 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -534,9 +534,10 @@ static struct audit_entry *audit_data_to_entry(struct
audit_rule_data *data,
entry-rule.buflen += f-val;
err = audit_make_tree(entry-rule, str, f-op);
- kfree(str);
- if (err)
+ if (err) {
+ kfree(str);
goto exit_free;
+ }
break;
case AUDIT_INODE:
err = audit_to_inode(entry-rule, f);
--
1.7.1
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH V5 1/5] audit: implement audit by executable
From: Eric Paris epa...@redhat.com
This patch implements the ability to filter on the executable. It is
clearly incomplete! This patch adds the inode/dev of the executable at
the moment the rule is loaded. It does not update if the executable is
updated/moved/whatever. That should be added. But at this moment, this
patch works.
Based-on-user-interface-by: Richard Guy Briggs r...@redhat.com
Cc: r...@redhat.com
Based-on-idea-by: Peter Moody pmo...@google.com
Cc: pmo...@google.com
Signed-off-by: Eric Paris epa...@redhat.com
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
include/linux/audit.h |1 +
include/uapi/linux/audit.h |2 +
kernel/Makefile|2 +-
kernel/audit.h | 32 +
kernel/audit_exe.c | 109
kernel/auditfilter.c | 44 ++
kernel/auditsc.c | 16 ++
7 files changed, 205 insertions(+), 1 deletions(-)
create mode 100644 kernel/audit_exe.c
diff --git a/include/linux/audit.h b/include/linux/audit.h
index 36dffec..ce51204 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -59,6 +59,7 @@ struct audit_krule {
struct audit_field *inode_f; /* quick access to an inode field */
struct audit_watch *watch; /* associated watch */
struct audit_tree *tree; /* associated watched tree */
+ struct audit_exe*exe;
struct list_headrlist; /* entry in audit_{watch,tree}.rules
list */
struct list_headlist; /* for AUDIT_LIST* purposes only */
u64 prio;
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4d100c8..101d344 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -266,6 +266,8 @@
#define AUDIT_OBJ_UID 109
#define AUDIT_OBJ_GID 110
#define AUDIT_FIELD_COMPARE111
+#define AUDIT_EXE 112
+#define AUDIT_EXE_CHILDREN 113
#define AUDIT_ARG0 200
#define AUDIT_ARG1 (AUDIT_ARG0+1)
diff --git a/kernel/Makefile b/kernel/Makefile
index f2a8b62..60def04 100644
--- a/kernel/Makefile
+++ b/kernel/Makefile
@@ -63,7 +63,7 @@ obj-$(CONFIG_SMP) += stop_machine.o
obj-$(CONFIG_KPROBES_SANITY_TEST) += test_kprobes.o
obj-$(CONFIG_AUDIT) += audit.o auditfilter.o
obj-$(CONFIG_AUDITSYSCALL) += auditsc.o
-obj-$(CONFIG_AUDIT_WATCH) += audit_watch.o
+obj-$(CONFIG_AUDIT_WATCH) += audit_watch.o audit_exe.o
obj-$(CONFIG_AUDIT_TREE) += audit_tree.o
obj-$(CONFIG_GCOV_KERNEL) += gcov/
obj-$(CONFIG_KPROBES) += kprobes.o
diff --git a/kernel/audit.h b/kernel/audit.h
index 3cdffad..7825c7e 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -56,6 +56,7 @@ enum audit_state {
/* Rule lists */
struct audit_watch;
+struct audit_exe;
struct audit_tree;
struct audit_chunk;
@@ -279,6 +280,13 @@ extern int audit_add_watch(struct audit_krule *krule,
struct list_head **list);
extern void audit_remove_watch_rule(struct audit_krule *krule);
extern char *audit_watch_path(struct audit_watch *watch);
extern int audit_watch_compare(struct audit_watch *watch, unsigned long ino,
dev_t dev);
+
+int audit_make_exe_rule(struct audit_krule *krule, char *pathname, int len,
u32 op);
+void audit_remove_exe_rule(struct audit_krule *krule);
+char *audit_exe_path(struct audit_exe *exe);
+int audit_dup_exe(struct audit_krule *new, struct audit_krule *old);
+int audit_exe_compare(struct task_struct *tsk, struct audit_exe *exe);
+
#else
#define audit_put_watch(w) {}
#define audit_get_watch(w) {}
@@ -288,6 +296,30 @@ extern int audit_watch_compare(struct audit_watch *watch,
unsigned long ino, dev
#define audit_watch_path(w)
#define audit_watch_compare(w, i, d) 0
+static inline int audit_make_exe_rule(struct audit_krule *krule, char
*pathname, int len, u32 op)
+{
+ return -EINVAL;
+}
+static inline void audit_remove_exe_rule(struct audit_krule *krule)
+{
+ BUG();
+ return 0;
+}
+static inline char *audit_exe_path(struct audit_exe *exe)
+{
+ BUG();
+ return ;
+}
+static inline int audit_dup_exe(struct audit_krule *new, struct audit_krule
*old)
+{
+ BUG();
+ return -EINVAL
+}
+static inline int audit_exe_compare(struct task_struct *tsk, struct audit_exe
*exe)
+{
+ BUG();
+ return 0;
+}
#endif /* CONFIG_AUDIT_WATCH */
#ifdef CONFIG_AUDIT_TREE
diff --git a/kernel/audit_exe.c b/kernel/audit_exe.c
new file mode 100644
index 000..ec3231b
--- /dev/null
+++ b/kernel/audit_exe.c
@@ -0,0 +1,109 @@
+/* audit_exe.c -- filtering of audit events
+ *
+ * Copyright 2014 Red Hat, Inc.
+ *
+ * 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;
[PATCH V5 3/5] audit: convert audit_exe to audit_fsnotify
From: Eric Paris epa...@redhat.com
Instead of just hard coding the ino and dev of the executable we care
about at the moment the rule is inserted into the kernel, use the new
audit_fsnotify infrastructure. This means that if the inode in question
is unlinked and creat'd (aka updated) the rule will just continue to
work.
Signed-off-by: Eric Paris epa...@redhat.com
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
include/linux/audit.h |2 +-
kernel/audit.h| 32 +++---
kernel/audit_exe.c| 87 +++--
kernel/auditfilter.c | 15 +---
4 files changed, 27 insertions(+), 109 deletions(-)
diff --git a/include/linux/audit.h b/include/linux/audit.h
index ce51204..0ffa268 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -59,7 +59,7 @@ struct audit_krule {
struct audit_field *inode_f; /* quick access to an inode field */
struct audit_watch *watch; /* associated watch */
struct audit_tree *tree; /* associated watched tree */
- struct audit_exe*exe;
+ struct audit_fsnotify_mark *exe;
struct list_headrlist; /* entry in audit_{watch,tree}.rules
list */
struct list_headlist; /* for AUDIT_LIST* purposes only */
u64 prio;
diff --git a/kernel/audit.h b/kernel/audit.h
index b8ecc06..9821732 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -57,7 +57,6 @@ enum audit_state {
/* Rule lists */
struct audit_watch;
struct audit_fsnotify_mark;
-struct audit_exe;
struct audit_tree;
struct audit_chunk;
@@ -288,11 +287,8 @@ char *audit_mark_path(struct audit_fsnotify_mark *mark);
void audit_remove_mark(struct audit_fsnotify_mark *audit_mark);
int audit_mark_compare(struct audit_fsnotify_mark *mark, unsigned long ino,
dev_t dev);
-int audit_make_exe_rule(struct audit_krule *krule, char *pathname, int len,
u32 op);
-void audit_remove_exe_rule(struct audit_krule *krule);
-char *audit_exe_path(struct audit_exe *exe);
int audit_dup_exe(struct audit_krule *new, struct audit_krule *old);
-int audit_exe_compare(struct task_struct *tsk, struct audit_exe *exe);
+int audit_exe_compare(struct task_struct *tsk, struct audit_fsnotify_mark
*mark);
#else
#define audit_put_watch(w) {}
@@ -319,36 +315,18 @@ static inline void audit_remove_mark(struct
audit_fsnotify_mark *audit_mark)
BUG();
}
-static inline int audit_mark_compare(struct audit_fsnotify_mark *mark,
unsigned long ino, dev_t dev)
+static inline int audit_exe_compare(struct task_struct *tsk, struct
audit_fsnotify_mark *mark)
{
BUG();
- return 0;
-}
-
-static inline int audit_make_exe_rule(struct audit_krule *krule, char
*pathname, int len, u32 op)
-{
return -EINVAL;
}
-static inline void audit_remove_exe_rule(struct audit_krule *krule)
-{
- BUG();
- return 0;
-}
-static inline char *audit_exe_path(struct audit_exe *exe)
-{
- BUG();
- return ;
-}
+
static inline int audit_dup_exe(struct audit_krule *new, struct audit_krule
*old)
{
BUG();
- return -EINVAL
-}
-static inline int audit_exe_compare(struct task_struct *tsk, struct audit_exe
*exe)
-{
- BUG();
- return 0;
+ return -EINVAL;
}
+
#endif /* CONFIG_AUDIT_WATCH */
#ifdef CONFIG_AUDIT_TREE
diff --git a/kernel/audit_exe.c b/kernel/audit_exe.c
index ec3231b..0c7ee8d 100644
--- a/kernel/audit_exe.c
+++ b/kernel/audit_exe.c
@@ -17,93 +17,30 @@
#include linux/kernel.h
#include linux/audit.h
-#include linux/mutex.h
#include linux/fs.h
#include linux/namei.h
#include linux/slab.h
#include audit.h
-struct audit_exe {
- char *pathname;
- unsigned long ino;
- dev_t dev;
-};
-
-/* Translate a watch string to kernel respresentation. */
-int audit_make_exe_rule(struct audit_krule *krule, char *pathname, int len,
u32 op)
-{
- struct audit_exe *exe;
- struct path path;
- struct dentry *dentry;
- unsigned long ino;
- dev_t dev;
-
- if (pathname[0] != '/' || pathname[len-1] == '/')
- return -EINVAL;
-
- dentry = kern_path_locked(pathname, path);
- if (IS_ERR(dentry))
- return PTR_ERR(dentry);
- mutex_unlock(path.dentry-d_inode-i_mutex);
-
- if (!dentry-d_inode)
- return -ENOENT;
- dev = dentry-d_inode-i_sb-s_dev;
- ino = dentry-d_inode-i_ino;
- dput(dentry);
-
- exe = kmalloc(sizeof(*exe), GFP_KERNEL);
- if (!exe)
- return -ENOMEM;
- exe-ino = ino;
- exe-dev = dev;
- exe-pathname = pathname;
- krule-exe = exe;
-
- return 0;
-}
-
-void audit_remove_exe_rule(struct audit_krule *krule)
-{
- struct audit_exe *exe;
-
- exe = krule-exe;
- krule-exe = NULL;
- kfree(exe-pathname);
- kfree(exe);
-}
-
-char *audit_exe_path(struct audit_exe *exe)
-{
- return exe-pathname;
-}
-
[PATCH V5 0/5] audit by executable name
This is a part of Peter Moody, my and Eric Paris' work to implement audit by executable name. Please see the accompanying userspace patch: https://www.redhat.com/archives/linux-audit/2014-May/msg00019.html The userspace interface is not expected to change appreciably unless something important has been overlooked. Setting and deleting rules works as expected. If the path does not exist at rule creation time, it will be re-evaluated every time there is a change to the parent directory at which point the change in device and inode will be noted. Here's a sample run: # /usr/local/sbin/auditctl -a always,exit -F dir=/tmp -F exe=/bin/touch -F key=touch_tmp # /usr/local/sbin/ausearch --start recent -k touch_tmp time-Mon Jun 30 14:15:06 2014 type=CONFIG_CHANGE msg=audit(1404152106.683:149): auid=0 ses=1 subj=unconfined_u :unconfined_r:auditctl_t:s0-s0:c0.c1023 op=add_rule key=touch_tmp list=4 res =1 # /usr/local/sbin/auditctl -l -a always,exit -S all -F dir=/tmp -F exe=/bin/touch -F key=touch_tmp # touch /tmp/test # /usr/local/sbin/ausearch --start recent -k touch_tmp time-Wed Jul 2 12:18:47 2014 type=UNKNOWN[1327] msg=audit(1404317927.319:132): proctitle=746F756368002F746D702F74657374 type=PATH msg=audit(1404317927.319:132): item=1 name=/tmp/test inode=25997 dev=00:20 mode=0100644 ouid=0 ogid=0 rdev=00:00 obj=unconfined_u:object_r:user_tmp_t:s0 nametype=CREATE type=PATH msg=audit(1404317927.319:132): item=0 name=/tmp/ inode=11144 dev=00:20 mode=041777 ouid=0 ogid=0 rdev=00:00 obj=system_u:object_r:tmp_t:s0 nametype=PARENT type=CWD msg=audit(1404317927.319:132): cwd=/root type=SYSCALL msg=audit(1404317927.319:132): arch=c03e syscall=2 success=yes exit=3 a0=7a403dd5 a1=941 a2=1b6 a3=34b65b2c6c items=2 ppid=4321 pid=6436 auid=0 uid=0 gid=0 euid=0 suid=0 fsuid=0 egid=0 sgid=0 fsgid=0 tty=ttyS0 ses=1 comm=touch exe=/usr/bin/touch subj=unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023 key=touch_tmp Revision history: v5: Revert patch Let audit_free_rule() take care of calling audit_remove_mark(). since it caused a group mark deadlock. v4: Re-order and squash down fixups Fix audit_dup_exe() to copy pathname string before calling audit_alloc_mark(). v3: Rationalize and rename some function names and clean up get/put and free code. Rename several watch references to mark. Rename audit_remove_rule() to audit_remove_mark_rule(). Let audit_free_rule() take care of calling audit_remove_mark(). Put audit_alloc_mark() arguments in same order as watch, tree and inode. Move the access to the entry for audit_match_signal() to the beginning of the function in case the entry found is the same one passed in. This will enable it to be used by audit_remove_mark_rule(). https://www.redhat.com/archives/linux-audit/2014-July/msg0.html v2: Misguided attempt to add in audit_exe similar to watches https://www.redhat.com/archives/linux-audit/2014-June/msg00066.html v1.5: eparis' switch to fsnotify https://www.redhat.com/archives/linux-audit/2014-May/msg00046.html https://www.redhat.com/archives/linux-audit/2014-May/msg00066.html v1: Change to path interface instead of inode https://www.redhat.com/archives/linux-audit/2014-May/msg00017.html v0: Peter Moodie's original patches https://www.redhat.com/archives/linux-audit/2012-August/msg00033.html Next step: Get full-path notify working. Eric Paris (3): audit: implement audit by executable audit: clean simple fsnotify implementation audit: convert audit_exe to audit_fsnotify Richard Guy Briggs (2): audit: avoid double copying the audit_exe path string Revert fixup! audit: clean simple fsnotify implementation include/linux/audit.h |1 + include/uapi/linux/audit.h |2 + kernel/Makefile|2 +- kernel/audit.h | 39 +++ kernel/audit_exe.c | 49 + kernel/audit_fsnotify.c| 237 kernel/auditfilter.c | 52 +- kernel/auditsc.c | 16 +++ 8 files changed, 395 insertions(+), 3 deletions(-) create mode 100644 kernel/audit_exe.c create mode 100644 kernel/audit_fsnotify.c -- Linux-audit mailing list Linux-audit@redhat.com https://www.redhat.com/mailman/listinfo/linux-audit
[PATCH V5 4/5] audit: avoid double copying the audit_exe path string
Make this interface consistent with watch and filter key, avoiding the extra
string copy and simply consume the new string pointer.
Signed-off-by: Richard Guy Briggs r...@redhat.com
---
kernel/audit_exe.c |5 -
kernel/audit_fsnotify.c | 12 ++--
kernel/auditfilter.c|2 +-
3 files changed, 7 insertions(+), 12 deletions(-)
diff --git a/kernel/audit_exe.c b/kernel/audit_exe.c
index 0c7ee8d..ff6e3d6 100644
--- a/kernel/audit_exe.c
+++ b/kernel/audit_exe.c
@@ -27,10 +27,13 @@ int audit_dup_exe(struct audit_krule *new, struct
audit_krule *old)
struct audit_fsnotify_mark *audit_mark;
char *pathname;
- pathname = audit_mark_path(old-exe);
+ pathname = kstrdup(audit_mark_path(old-exe), GFP_KERNEL);
+ if (!pathname)
+ return -ENOMEM;
audit_mark = audit_alloc_mark(new, pathname, strlen(pathname));
if (IS_ERR(audit_mark))
+ kfree(pathname);
return PTR_ERR(audit_mark);
new-exe = audit_mark;
diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c
index f4b3e66..6daf5c3 100644
--- a/kernel/audit_fsnotify.c
+++ b/kernel/audit_fsnotify.c
@@ -94,7 +94,6 @@ struct audit_fsnotify_mark *audit_alloc_mark(struct
audit_krule *krule, char *pa
struct dentry *dentry;
struct inode *inode;
unsigned long ino;
- char *local_pathname;
dev_t dev;
int ret;
@@ -115,20 +114,13 @@ struct audit_fsnotify_mark *audit_alloc_mark(struct
audit_krule *krule, char *pa
ino = dentry-d_inode-i_ino;
}
- audit_mark = ERR_PTR(-ENOMEM);
- local_pathname = kstrdup(pathname, GFP_KERNEL);
- if (!local_pathname)
- goto out;
-
audit_mark = kzalloc(sizeof(*audit_mark), GFP_KERNEL);
- if (unlikely(!audit_mark)) {
- kfree(local_pathname);
+ if (unlikely(!audit_mark))
goto out;
- }
fsnotify_init_mark(audit_mark-mark, audit_free_fsnotify_mark);
audit_mark-mark.mask = AUDIT_FS_EVENTS;
- audit_mark-path = local_pathname;
+ audit_mark-path = pathname;
audit_mark-ino = ino;
audit_mark-dev = dev;
audit_mark-rule = krule;
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index fff92cf..570e79a 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -575,8 +575,8 @@ static struct audit_entry *audit_data_to_entry(struct
audit_rule_data *data,
entry-rule.buflen += f-val;
audit_mark = audit_alloc_mark(entry-rule, str,
f-val);
- kfree(str);
if (IS_ERR(audit_mark)) {
+ kfree(str);
err = PTR_ERR(audit_mark);
goto exit_free;
}
--
1.7.1
--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit
