[PATCH ghak59 V2 0/6] audit: config_change normalizations and event record gathering

[PATCH ghak59 V2 0/6] audit: config_change normalizations and event record gathering

[Richard Guy Briggs]

Make a number of changes to normalize CONFIG_CHANGE records by adding
missing op= fields, providing more information in existing op fields and
connecting all records to existing audit events.

The user record patch is included but is *optional* since there is doubt
that we want to disconnect the records from a single event.

Since tree purge records are processed after the EOE record is produced,
the order of operation of the EOE record and the purge will have to be
reversed so that the purge records can be included in the event.

For reference, here are the calling methods and function tree for all
CONFIG_CHANGE events:
- audit_log_config_change() "op=set"
        - AUDIT_SET:AUDIT_STATUS_PID
        - AUDIT_SET:AUDIT_STATUS_LOST
        - audit_do_config_change()
                - AUDIT_SET:AUDIT_STATUS_FAILURE
                - AUDIT_SET:AUDIT_STATUS_ENABLED
                - AUDIT_SET:AUDIT_STATUS_RATE_LIMIT
                - AUDIT_SET:AUDIT_STATUS_BACKLOG_LIMIT
                - AUDIT_SET:AUDIT_STATUS_BACKLOG_WAIT_TIME
- audit_log_common_recv_msg()
        - AUDIT_*USER* events (not CONFIG_CHANGE like all the rest)
        - AUDIT_LOCKED "op=%s_rule"(add/remove)
        - AUDIT_TRIM "op=trim"
        - AUDIT_MAKE_EQUIV: "op=make_equiv"
        - AUDIT_TTY_SET: "op=tty_set"
- audit_log_rule_change()
        - AUDIT_ADD_RULE -F dir=:
        - AUDIT_DEL_RULE -F dir=:
- audit_mark_log_rule_change()
        - audit_autoremove_mark_rule() "op=autoremove_rule(mark)"
                - audit_mark_handle_event()
                        - audit_mark_fsnotify_ops.handle_event
- audit_tree_log_remove_rule() "op=remove_rule(tree:%s)" from kill_rules()
        - from trim_marked()
                - AUDIT_TRIM: audit_trim_trees() "trim"
                - audit_add_tree_rule() iterate_mounts err "add"
                        - audit_add_rule()
                                - audit_rule_change()
                                        - AUDIT_ADD_RULE -F dir=:
                - AUDIT_MAKE_EQUIV: audit_tag_tree() iterate_mounts err "equiv"
        - from audit_kill_trees()
                - __audit_free() "free"
                        - do_exit()
                        - copy_process() err
                - __audit_syscall_exit() "exit"
        - from evict_chunk() "evict"
                - audit_tree_freeing_mark()
                        - audit_tree_ops.freeing_mark
- audit_watch_log_rule_change()
        - audit_update_watch() "updated_rules(watch:inval)" : "updated_rules(watch:set)"
                - audit_watch_handle_event() FS_CREATE|FS_MOVED_TO, FS_DELETE|FS_MOVED_FROM
                        - audit_watch_fsnotify_ops.handle_event
        - audit_remove_parent_watches() "remove_rule(watch:parent)"
                - audit_watch_handle_event() FS_DELETE_SELF|FS_UNMOUNT|FS_MOVE_SELF
                        - audit_watch_fsnotify_ops.handle_event

See: https://github.com/linux-audit/audit-kernel/issues/50
See: https://github.com/linux-audit/audit-kernel/issues/59

Changelog:
v2:
- re-order audit_log_exit() and audit_kill_trees()
- drop EOE reordering patch
- rebase on 4.18-rc1 (audit/next)

Richard Guy Briggs (6):
  audit: give a clue what CONFIG_CHANGE op was involved
  audit: add syscall information to CONFIG_CHANGE records
  audit: exclude user records from syscall context
  audit: hand taken context to audit_kill_trees for syscall logging
  audit: kill trees before logging syscall exit for exit/free
  audit: extend config_change mark/watch/tree rule changes

 kernel/audit.c          | 20 ++++++++++++++------
 kernel/audit.h          |  4 ++--
 kernel/audit_fsnotify.c |  4 ++--
 kernel/audit_tree.c     | 28 +++++++++++++++-------------
 kernel/audit_watch.c    |  8 +++++---
 kernel/auditfilter.c    |  2 +-
 kernel/auditsc.c        |  9 ++++-----
 7 files changed, 43 insertions(+), 32 deletions(-)

-- 
1.8.3.1

[PATCH ghak59 V2 1/6] audit: give a clue what CONFIG_CHANGE op was involved

[Richard Guy Briggs]

The failure to add an audit rule due to audit locked gives no clue
what CONFIG_CHANGE operation failed.
Similarly the set operation is the only other operation that doesn't
give the "op=" field to indicate the action.
All other CONFIG_CHANGE records include an op= field to give a clue as
to what sort of configuration change is being executed.

Since these are the only CONFIG_CHANGE records that that do not have an
op= field, add them to bring them in line with the rest.

Old records:
type=CONFIG_CHANGE msg=audit(1519812997.781:374): pid=610 uid=0 auid=0 ses=1 subj=... audit_enabled=2 res=0
type=CONFIG_CHANGE msg=audit(2018-06-14 14:55:04.507:47) : audit_enabled=1 old=1 auid=unset ses=unset subj=... res=yes

New records:
type=CONFIG_CHANGE msg=audit(1520958477.855:100): pid=610 uid=0 auid=0 ses=1 subj=... op=add_rule audit_enabled=2 res=0

type=CONFIG_CHANGE msg=audit(2018-06-14 14:55:04.507:47) : op=set audit_enabled=1 old=1 auid=unset ses=unset subj=... res=yes

See: https://github.com/linux-audit/audit-kernel/issues/59
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 2a80587..1ed8723 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -400,7 +400,7 @@ static int audit_log_config_change(char *function_name, u32 new, u32 old,
 	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
 	if (unlikely(!ab))
 		return rc;
-	audit_log_format(ab, "%s=%u old=%u", function_name, new, old);
+	audit_log_format(ab, "op=set %s=%u old=%u", function_name, new, old);
 	audit_log_session_info(ab);
 	rc = audit_log_task_context(ab);
 	if (rc)
@@ -1362,7 +1362,9 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 			return -EINVAL;
 		if (audit_enabled == AUDIT_LOCKED) {
 			audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE);
-			audit_log_format(ab, " audit_enabled=%d res=0", audit_enabled);
+			audit_log_format(ab, " op=%s_rule audit_enabled=%d res=0",
+					 msg_type == AUDIT_ADD_RULE ? "add" : "remove",
+					 audit_enabled);
 			audit_log_end(ab);
 			return -EPERM;
 		}
-- 
1.8.3.1

[PATCH ghak59 V2 2/6] audit: add syscall information to CONFIG_CHANGE records

[Richard Guy Briggs]

Tie syscall information to all CONFIG_CHANGE calls since they are all a
result of user actions.

See: https://github.com/linux-audit/audit-kernel/issues/59
See: https://github.com/linux-audit/audit-kernel/issues/50
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit.c          | 4 ++--
 kernel/audit_fsnotify.c | 2 +-
 kernel/audit_tree.c     | 2 +-
 kernel/audit_watch.c    | 2 +-
 kernel/auditfilter.c    | 2 +-
 5 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 1ed8723..6cfe3c9 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -397,7 +397,7 @@ static int audit_log_config_change(char *function_name, u32 new, u32 old,
 	struct audit_buffer *ab;
 	int rc = 0;
 
-	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
+	ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_CONFIG_CHANGE);
 	if (unlikely(!ab))
 		return rc;
 	audit_log_format(ab, "op=set %s=%u old=%u", function_name, new, old);
@@ -1064,7 +1064,7 @@ static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
 		return;
 	}
 
-	*ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
+	*ab = audit_log_start(audit_context(), GFP_KERNEL, msg_type);
 	if (unlikely(!*ab))
 		return;
 	audit_log_format(*ab, "pid=%d uid=%u", pid, uid);
diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c
index fba7804..ae5c89b 100644
--- a/kernel/audit_fsnotify.c
+++ b/kernel/audit_fsnotify.c
@@ -127,7 +127,7 @@ static void audit_mark_log_rule_change(struct audit_fsnotify_mark *audit_mark, c
 
 	if (!audit_enabled)
 		return;
-	ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
+	ab = audit_log_start(audit_context(), GFP_NOFS, AUDIT_CONFIG_CHANGE);
 	if (unlikely(!ab))
 		return;
 	audit_log_format(ab, "auid=%u ses=%u op=%s",
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index 9f6eaeb..f0b7d30 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -499,7 +499,7 @@ static void audit_tree_log_remove_rule(struct audit_krule *rule)
 
 	if (!audit_enabled)
 		return;
-	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
+	ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_CONFIG_CHANGE);
 	if (unlikely(!ab))
 		return;
 	audit_log_format(ab, "op=remove_rule");
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index 787c7af..26c2fae 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -242,7 +242,7 @@ static void audit_watch_log_rule_change(struct audit_krule *r, struct audit_watc
 
 	if (!audit_enabled)
 		return;
-	ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
+	ab = audit_log_start(audit_context(), GFP_NOFS, AUDIT_CONFIG_CHANGE);
 	if (!ab)
 		return;
 	audit_log_format(ab, "auid=%u ses=%u op=%s",
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index bf309f2..26a80a9 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -1091,7 +1091,7 @@ static void audit_log_rule_change(char *action, struct audit_krule *rule, int re
 	if (!audit_enabled)
 		return;
 
-	ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
+	ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_CONFIG_CHANGE);
 	if (!ab)
 		return;
 	audit_log_session_info(ab);
-- 
1.8.3.1

[PATCH ghak59 V2 3/6] audit: exclude user records from syscall context

[Richard Guy Briggs]

Since the function audit_log_common_recv_msg() is shared by a number of
AUDIT_CONFIG_CHANGE and the entire range of AUDIT_USER_* record types,
and since the AUDIT_CONFIG_CHANGE message type has been converted to a
syscall accompanied record type, special-case the AUDIT_USER_* range of
messages so they remain standalone records.

See: https://github.com/linux-audit/audit-kernel/issues/59
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/kernel/audit.c b/kernel/audit.c
index 6cfe3c9..62ada1b 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1054,7 +1054,8 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
 	return err;
 }
 
-static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
+static void __audit_log_common_recv_msg(struct audit_context *context,
+					struct audit_buffer **ab, u16 msg_type)
 {
 	uid_t uid = from_kuid(&init_user_ns, current_uid());
 	pid_t pid = task_tgid_nr(current);
@@ -1064,7 +1065,7 @@ static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
 		return;
 	}
 
-	*ab = audit_log_start(audit_context(), GFP_KERNEL, msg_type);
+	*ab = audit_log_start(context, GFP_KERNEL, msg_type);
 	if (unlikely(!*ab))
 		return;
 	audit_log_format(*ab, "pid=%d uid=%u", pid, uid);
@@ -1072,6 +1073,11 @@ static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
 	audit_log_task_context(*ab);
 }
 
+static inline void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
+{
+	__audit_log_common_recv_msg(audit_context(), ab, msg_type);
+}
+
 int is_audit_feature_set(int i)
 {
 	return af.features & AUDIT_FEATURE_TO_MASK(i);
@@ -1338,7 +1344,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
 				if (err)
 					break;
 			}
-			audit_log_common_recv_msg(&ab, msg_type);
+			__audit_log_common_recv_msg(NULL, &ab, msg_type);
 			if (msg_type != AUDIT_USER_TTY)
 				audit_log_format(ab, " msg='%.*s'",
 						 AUDIT_MESSAGE_TEXT_MAX,
-- 
1.8.3.1

[PATCH ghak59 V2 4/6] audit: hand taken context to audit_kill_trees for syscall logging

[Richard Guy Briggs]

Since the context is taken from the task in __audit_syscall_exit() and
__audit_free(), hand it to audit_kill_trees() so it can be used to
associate with a syscall record.  This requires adding the context
parameter to kill_rules() rather than using the current audit_context
(which has been taken).

The callers of trim_marked() and evict_chunk() still have their context.

See: https://github.com/linux-audit/audit-kernel/issues/50
See: https://github.com/linux-audit/audit-kernel/issues/59
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit.h      |  4 ++--
 kernel/audit_tree.c | 18 ++++++++++--------
 kernel/auditsc.c    |  4 ++--
 3 files changed, 14 insertions(+), 12 deletions(-)

diff --git a/kernel/audit.h b/kernel/audit.h
index 214e149..f39f7aa 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -312,7 +312,7 @@ extern void audit_log_d_path_exe(struct audit_buffer *ab,
 extern int audit_tag_tree(char *old, char *new);
 extern const char *audit_tree_path(struct audit_tree *tree);
 extern void audit_put_tree(struct audit_tree *tree);
-extern void audit_kill_trees(struct list_head *list);
+extern void audit_kill_trees(struct audit_context *context);
 #else
 #define audit_remove_tree_rule(rule) BUG()
 #define audit_add_tree_rule(rule) -EINVAL
@@ -321,7 +321,7 @@ extern void audit_log_d_path_exe(struct audit_buffer *ab,
 #define audit_put_tree(tree) (void)0
 #define audit_tag_tree(old, new) -EINVAL
 #define audit_tree_path(rule) ""	/* never called */
-#define audit_kill_trees(list) BUG()
+#define audit_kill_trees(context) BUG()
 #endif
 
 extern char *audit_unpack_string(void **bufp, size_t *remain, size_t len);
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index f0b7d30..c2281e3 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -493,13 +493,13 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
 	return 0;
 }
 
-static void audit_tree_log_remove_rule(struct audit_krule *rule)
+static void audit_tree_log_remove_rule(struct audit_context *context, struct audit_krule *rule)
 {
 	struct audit_buffer *ab;
 
 	if (!audit_enabled)
 		return;
-	ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_CONFIG_CHANGE);
+	ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
 	if (unlikely(!ab))
 		return;
 	audit_log_format(ab, "op=remove_rule");
@@ -510,7 +510,7 @@ static void audit_tree_log_remove_rule(struct audit_krule *rule)
 	audit_log_end(ab);
 }
 
-static void kill_rules(struct audit_tree *tree)
+static void kill_rules(struct audit_context *context, struct audit_tree *tree)
 {
 	struct audit_krule *rule, *next;
 	struct audit_entry *entry;
@@ -521,7 +521,7 @@ static void kill_rules(struct audit_tree *tree)
 		list_del_init(&rule->rlist);
 		if (rule->tree) {
 			/* not a half-baked one */
-			audit_tree_log_remove_rule(rule);
+			audit_tree_log_remove_rule(context, rule);
 			if (entry->rule.exe)
 				audit_remove_mark(entry->rule.exe);
 			rule->tree = NULL;
@@ -584,7 +584,7 @@ static void trim_marked(struct audit_tree *tree)
 		tree->goner = 1;
 		spin_unlock(&hash_lock);
 		mutex_lock(&audit_filter_mutex);
-		kill_rules(tree);
+		kill_rules(audit_context(), tree);
 		list_del_init(&tree->list);
 		mutex_unlock(&audit_filter_mutex);
 		prune_one(tree);
@@ -924,8 +924,10 @@ static void audit_schedule_prune(void)
  * ... and that one is done if evict_chunk() decides to delay until the end
  * of syscall.  Runs synchronously.
  */
-void audit_kill_trees(struct list_head *list)
+void audit_kill_trees(struct audit_context *context)
 {
+	struct list_head *list = &context->killed_trees;
+
 	audit_ctl_lock();
 	mutex_lock(&audit_filter_mutex);
 
@@ -933,7 +935,7 @@ void audit_kill_trees(struct list_head *list)
 		struct audit_tree *victim;
 
 		victim = list_entry(list->next, struct audit_tree, list);
-		kill_rules(victim);
+		kill_rules(context, victim);
 		list_del_init(&victim->list);
 
 		mutex_unlock(&audit_filter_mutex);
@@ -972,7 +974,7 @@ static void evict_chunk(struct audit_chunk *chunk)
 		list_del_init(&owner->same_root);
 		spin_unlock(&hash_lock);
 		if (!postponed) {
-			kill_rules(owner);
+			kill_rules(audit_context(), owner);
 			list_move(&owner->list, &prune_list);
 			need_prune = 1;
 		} else {
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index fb20746..986c5ce 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1490,7 +1490,7 @@ void __audit_free(struct task_struct *tsk)
 	if (context->in_syscall && context->current_state == AUDIT_RECORD_CONTEXT)
 		audit_log_exit(context, tsk);
 	if (!list_empty(&context->killed_trees))
-		audit_kill_trees(&context->killed_trees);
+		audit_kill_trees(context);
 
 	audit_free_context(context);
 }
@@ -1577,7 +1577,7 @@ void __audit_syscall_exit(int success, long return_code)
 	context->prio = context->state == AUDIT_RECORD_CONTEXT ? ~0ULL : 0;
 
 	if (!list_empty(&context->killed_trees))
-		audit_kill_trees(&context->killed_trees);
+		audit_kill_trees(context);
 
 	audit_free_names(context);
 	unroll_tree_refs(context, NULL, 0);
-- 
1.8.3.1

[PATCH ghak59 V2 5/6] audit: kill trees before logging syscall exit for exit/free

[Richard Guy Briggs]

The EOE record was being issued prior to the pruning of the killed_tree
list.

Move the kill_trees call before the audit_log_exit call in
__audit_free() and __audit_syscall_exit() so that the user library
doesn't leave out any appended pruned trees CONFIG_CHANGE records due to
the EOE flagging the end of the event.

See: https://github.com/linux-audit/audit-kernel/issues/50
See: https://github.com/linux-audit/audit-kernel/issues/59
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/auditsc.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 986c5ce..55fd2a3 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1482,6 +1482,8 @@ void __audit_free(struct task_struct *tsk)
 	if (!context)
 		return;
 
+	if (!list_empty(&context->killed_trees))
+		audit_kill_trees(context);
 	/* Check for system calls that do not go through the exit
 	 * function (e.g., exit_group), then free context block.
 	 * We use GFP_ATOMIC here because we might be doing this
@@ -1489,8 +1491,6 @@ void __audit_free(struct task_struct *tsk)
 	/* that can happen only if we are called from do_exit() */
 	if (context->in_syscall && context->current_state == AUDIT_RECORD_CONTEXT)
 		audit_log_exit(context, tsk);
-	if (!list_empty(&context->killed_trees))
-		audit_kill_trees(context);
 
 	audit_free_context(context);
 }
@@ -1570,15 +1570,14 @@ void __audit_syscall_exit(int success, long return_code)
 	if (!context)
 		return;
 
+	if (!list_empty(&context->killed_trees))
+		audit_kill_trees(context);
 	if (context->in_syscall && context->current_state == AUDIT_RECORD_CONTEXT)
 		audit_log_exit(context, current);
 
 	context->in_syscall = 0;
 	context->prio = context->state == AUDIT_RECORD_CONTEXT ? ~0ULL : 0;
 
-	if (!list_empty(&context->killed_trees))
-		audit_kill_trees(context);
-
 	audit_free_names(context);
 	unroll_tree_refs(context, NULL, 0);
 	audit_free_aux(context);
-- 
1.8.3.1

[PATCH ghak59 V2 6/6] audit: extend config_change mark/watch/tree rule changes

[Richard Guy Briggs]

Give a clue as to the source of mark, watch and tree rule changes.

See: https://github.com/linux-audit/audit-kernel/issues/50
See: https://github.com/linux-audit/audit-kernel/issues/59
Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 kernel/audit.h          |  4 ++--
 kernel/audit_fsnotify.c |  2 +-
 kernel/audit_tree.c     | 24 ++++++++++++------------
 kernel/audit_watch.c    |  6 ++++--
 kernel/auditsc.c        |  4 ++--
 5 files changed, 21 insertions(+), 19 deletions(-)

diff --git a/kernel/audit.h b/kernel/audit.h
index f39f7aa..5e072f5 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -312,7 +312,7 @@ extern void audit_log_d_path_exe(struct audit_buffer *ab,
 extern int audit_tag_tree(char *old, char *new);
 extern const char *audit_tree_path(struct audit_tree *tree);
 extern void audit_put_tree(struct audit_tree *tree);
-extern void audit_kill_trees(struct audit_context *context);
+extern void audit_kill_trees(struct audit_context *context, char *trig);
 #else
 #define audit_remove_tree_rule(rule) BUG()
 #define audit_add_tree_rule(rule) -EINVAL
@@ -321,7 +321,7 @@ extern void audit_log_d_path_exe(struct audit_buffer *ab,
 #define audit_put_tree(tree) (void)0
 #define audit_tag_tree(old, new) -EINVAL
 #define audit_tree_path(rule) ""	/* never called */
-#define audit_kill_trees(context) BUG()
+#define audit_kill_trees(context, trig) BUG()
 #endif
 
 extern char *audit_unpack_string(void **bufp, size_t *remain, size_t len);
diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c
index ae5c89b..d52bb79 100644
--- a/kernel/audit_fsnotify.c
+++ b/kernel/audit_fsnotify.c
@@ -158,7 +158,7 @@ static void audit_autoremove_mark_rule(struct audit_fsnotify_mark *audit_mark)
 	struct audit_krule *rule = audit_mark->rule;
 	struct audit_entry *entry = container_of(rule, struct audit_entry, rule);
 
-	audit_mark_log_rule_change(audit_mark, "autoremove_rule");
+	audit_mark_log_rule_change(audit_mark, "autoremove_rule(mark)");
 	audit_del_rule(entry);
 }
 
diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index c2281e3..2035097 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -493,7 +493,7 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
 	return 0;
 }
 
-static void audit_tree_log_remove_rule(struct audit_context *context, struct audit_krule *rule)
+static void audit_tree_log_remove_rule(struct audit_context *context, struct audit_krule *rule, char *trig)
 {
 	struct audit_buffer *ab;
 
@@ -502,7 +502,7 @@ static void audit_tree_log_remove_rule(struct audit_context *context, struct aud
 	ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
 	if (unlikely(!ab))
 		return;
-	audit_log_format(ab, "op=remove_rule");
+	audit_log_format(ab, "op=remove_rule(tree:%s)", trig);
 	audit_log_format(ab, " dir=");
 	audit_log_untrustedstring(ab, rule->tree->pathname);
 	audit_log_key(ab, rule->filterkey);
@@ -510,7 +510,7 @@ static void audit_tree_log_remove_rule(struct audit_context *context, struct aud
 	audit_log_end(ab);
 }
 
-static void kill_rules(struct audit_context *context, struct audit_tree *tree)
+static void kill_rules(struct audit_context *context, struct audit_tree *tree, char *trig)
 {
 	struct audit_krule *rule, *next;
 	struct audit_entry *entry;
@@ -521,7 +521,7 @@ static void kill_rules(struct audit_context *context, struct audit_tree *tree)
 		list_del_init(&rule->rlist);
 		if (rule->tree) {
 			/* not a half-baked one */
-			audit_tree_log_remove_rule(context, rule);
+			audit_tree_log_remove_rule(context, rule, trig);
 			if (entry->rule.exe)
 				audit_remove_mark(entry->rule.exe);
 			rule->tree = NULL;
@@ -551,7 +551,7 @@ static void prune_one(struct audit_tree *victim)
 
 /* trim the uncommitted chunks from tree */
 
-static void trim_marked(struct audit_tree *tree)
+static void trim_marked(struct audit_tree *tree, char *trig)
 {
 	struct list_head *p, *q;
 	spin_lock(&hash_lock);
@@ -584,7 +584,7 @@ static void trim_marked(struct audit_tree *tree)
 		tree->goner = 1;
 		spin_unlock(&hash_lock);
 		mutex_lock(&audit_filter_mutex);
-		kill_rules(audit_context(), tree);
+		kill_rules(audit_context(), tree, trig);
 		list_del_init(&tree->list);
 		mutex_unlock(&audit_filter_mutex);
 		prune_one(tree);
@@ -665,7 +665,7 @@ void audit_trim_trees(void)
 				node->index &= ~(1U<<31);
 		}
 		spin_unlock(&hash_lock);
-		trim_marked(tree);
+		trim_marked(tree, "trim");
 		drop_collected_mounts(root_mnt);
 skip_it:
 		put_tree(tree);
@@ -798,7 +798,7 @@ int audit_add_tree_rule(struct audit_krule *rule)
 			node->index &= ~(1U<<31);
 		spin_unlock(&hash_lock);
 	} else {
-		trim_marked(tree);
+		trim_marked(tree, "add");
 		goto Err;
 	}
 
@@ -900,7 +900,7 @@ int audit_tag_tree(char *old, char *new)
 				node->index &= ~(1U<<31);
 			spin_unlock(&hash_lock);
 		} else {
-			trim_marked(tree);
+			trim_marked(tree, "equiv");
 		}
 
 		put_tree(tree);
@@ -924,7 +924,7 @@ static void audit_schedule_prune(void)
  * ... and that one is done if evict_chunk() decides to delay until the end
  * of syscall.  Runs synchronously.
  */
-void audit_kill_trees(struct audit_context *context)
+void audit_kill_trees(struct audit_context *context, char *trig)
 {
 	struct list_head *list = &context->killed_trees;
 
@@ -935,7 +935,7 @@ void audit_kill_trees(struct audit_context *context)
 		struct audit_tree *victim;
 
 		victim = list_entry(list->next, struct audit_tree, list);
-		kill_rules(context, victim);
+		kill_rules(context, victim, trig);
 		list_del_init(&victim->list);
 
 		mutex_unlock(&audit_filter_mutex);
@@ -974,7 +974,7 @@ static void evict_chunk(struct audit_chunk *chunk)
 		list_del_init(&owner->same_root);
 		spin_unlock(&hash_lock);
 		if (!postponed) {
-			kill_rules(audit_context(), owner);
+			kill_rules(audit_context(), owner, "evict");
 			list_move(&owner->list, &prune_list);
 			need_prune = 1;
 		} else {
diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
index 26c2fae..6f1e060 100644
--- a/kernel/audit_watch.c
+++ b/kernel/audit_watch.c
@@ -317,7 +317,9 @@ static void audit_update_watch(struct audit_parent *parent,
 			if (oentry->rule.exe)
 				audit_remove_mark(oentry->rule.exe);
 
-			audit_watch_log_rule_change(r, owatch, "updated_rules");
+			audit_watch_log_rule_change(r, owatch, invalidating ?
+						    "updated_rules(watch:inval)" :
+						    "updated_rules(watch:set)");
 
 			call_rcu(&oentry->rcu, audit_free_rule_rcu);
 		}
@@ -345,7 +347,7 @@ static void audit_remove_parent_watches(struct audit_parent *parent)
 	list_for_each_entry_safe(w, nextw, &parent->watches, wlist) {
 		list_for_each_entry_safe(r, nextr, &w->rules, rlist) {
 			e = container_of(r, struct audit_entry, rule);
-			audit_watch_log_rule_change(r, w, "remove_rule");
+			audit_watch_log_rule_change(r, w, "remove_rule(watch:parent)");
 			if (e->rule.exe)
 				audit_remove_mark(e->rule.exe);
 			list_del(&r->rlist);
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 55fd2a3..bb56c3e 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -1483,7 +1483,7 @@ void __audit_free(struct task_struct *tsk)
 		return;
 
 	if (!list_empty(&context->killed_trees))
-		audit_kill_trees(context);
+		audit_kill_trees(context, "free");
 	/* Check for system calls that do not go through the exit
 	 * function (e.g., exit_group), then free context block.
 	 * We use GFP_ATOMIC here because we might be doing this
@@ -1571,7 +1571,7 @@ void __audit_syscall_exit(int success, long return_code)
 		return;
 
 	if (!list_empty(&context->killed_trees))
-		audit_kill_trees(context);
+		audit_kill_trees(context, "exit");
 	if (context->in_syscall && context->current_state == AUDIT_RECORD_CONTEXT)
 		audit_log_exit(context, current);
 
-- 
1.8.3.1

Re: [PATCH ghak59 V2 1/6] audit: give a clue what CONFIG_CHANGE op was involved

[Paul Moore]

On Fri, Jul 27, 2018 at 3:51 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> The failure to add an audit rule due to audit locked gives no clue
> what CONFIG_CHANGE operation failed.
> Similarly the set operation is the only other operation that doesn't
> give the "op=" field to indicate the action.
> All other CONFIG_CHANGE records include an op= field to give a clue as
> to what sort of configuration change is being executed.
>
> Since these are the only CONFIG_CHANGE records that that do not have an
> op= field, add them to bring them in line with the rest.
>
> Old records:
> type=CONFIG_CHANGE msg=audit(1519812997.781:374): pid=610 uid=0 auid=0 ses=1 subj=... audit_enabled=2 res=0
> type=CONFIG_CHANGE msg=audit(2018-06-14 14:55:04.507:47) : audit_enabled=1 old=1 auid=unset ses=unset subj=... res=yes
>
> New records:
> type=CONFIG_CHANGE msg=audit(1520958477.855:100): pid=610 uid=0 auid=0 ses=1 subj=... op=add_rule audit_enabled=2 res=0
>
> type=CONFIG_CHANGE msg=audit(2018-06-14 14:55:04.507:47) : op=set audit_enabled=1 old=1 auid=unset ses=unset subj=... res=yes
>
> See: https://github.com/linux-audit/audit-kernel/issues/59
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/audit.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 2a80587..1ed8723 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -400,7 +400,7 @@ static int audit_log_config_change(char *function_name, u32 new, u32 old,
>         ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
>         if (unlikely(!ab))
>                 return rc;
> -       audit_log_format(ab, "%s=%u old=%u", function_name, new, old);
> +       audit_log_format(ab, "op=set %s=%u old=%u", function_name, new, old);
>         audit_log_session_info(ab);
>         rc = audit_log_task_context(ab);
>         if (rc)
> @@ -1362,7 +1362,9 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>                         return -EINVAL;
>                 if (audit_enabled == AUDIT_LOCKED) {
>                         audit_log_common_recv_msg(&ab, AUDIT_CONFIG_CHANGE);
> -                       audit_log_format(ab, " audit_enabled=%d res=0", audit_enabled);
> +                       audit_log_format(ab, " op=%s_rule audit_enabled=%d res=0",
> +                                        msg_type == AUDIT_ADD_RULE ? "add" : "remove",

Let's not be clever here, this just makes it harder to grep for
"op=add_rule"; make the ternary statement return "add_rule" or
"remove_rule".

> +                                        audit_enabled);
>                         audit_log_end(ab);
>                         return -EPERM;
>                 }

--
paul moore
www.paul-moore.com

Re: [PATCH ghak59 V2 2/6] audit: add syscall information to CONFIG_CHANGE records

[Paul Moore]

On Fri, Jul 27, 2018 at 3:51 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> Tie syscall information to all CONFIG_CHANGE calls since they are all a
> result of user actions.
>
> See: https://github.com/linux-audit/audit-kernel/issues/59
> See: https://github.com/linux-audit/audit-kernel/issues/50
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/audit.c          | 4 ++--
>  kernel/audit_fsnotify.c | 2 +-
>  kernel/audit_tree.c     | 2 +-
>  kernel/audit_watch.c    | 2 +-
>  kernel/auditfilter.c    | 2 +-
>  5 files changed, 6 insertions(+), 6 deletions(-)

See my comments in patch 3/6.

> diff --git a/kernel/audit.c b/kernel/audit.c
> index 1ed8723..6cfe3c9 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -397,7 +397,7 @@ static int audit_log_config_change(char *function_name, u32 new, u32 old,
>         struct audit_buffer *ab;
>         int rc = 0;
>
> -       ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
> +       ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_CONFIG_CHANGE);
>         if (unlikely(!ab))
>                 return rc;
>         audit_log_format(ab, "op=set %s=%u old=%u", function_name, new, old);
> @@ -1064,7 +1064,7 @@ static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
>                 return;
>         }
>
> -       *ab = audit_log_start(NULL, GFP_KERNEL, msg_type);
> +       *ab = audit_log_start(audit_context(), GFP_KERNEL, msg_type);
>         if (unlikely(!*ab))
>                 return;
>         audit_log_format(*ab, "pid=%d uid=%u", pid, uid);
> diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c
> index fba7804..ae5c89b 100644
> --- a/kernel/audit_fsnotify.c
> +++ b/kernel/audit_fsnotify.c
> @@ -127,7 +127,7 @@ static void audit_mark_log_rule_change(struct audit_fsnotify_mark *audit_mark, c
>
>         if (!audit_enabled)
>                 return;
> -       ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
> +       ab = audit_log_start(audit_context(), GFP_NOFS, AUDIT_CONFIG_CHANGE);
>         if (unlikely(!ab))
>                 return;
>         audit_log_format(ab, "auid=%u ses=%u op=%s",
> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> index 9f6eaeb..f0b7d30 100644
> --- a/kernel/audit_tree.c
> +++ b/kernel/audit_tree.c
> @@ -499,7 +499,7 @@ static void audit_tree_log_remove_rule(struct audit_krule *rule)
>
>         if (!audit_enabled)
>                 return;
> -       ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
> +       ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_CONFIG_CHANGE);
>         if (unlikely(!ab))
>                 return;
>         audit_log_format(ab, "op=remove_rule");
> diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
> index 787c7af..26c2fae 100644
> --- a/kernel/audit_watch.c
> +++ b/kernel/audit_watch.c
> @@ -242,7 +242,7 @@ static void audit_watch_log_rule_change(struct audit_krule *r, struct audit_watc
>
>         if (!audit_enabled)
>                 return;
> -       ab = audit_log_start(NULL, GFP_NOFS, AUDIT_CONFIG_CHANGE);
> +       ab = audit_log_start(audit_context(), GFP_NOFS, AUDIT_CONFIG_CHANGE);
>         if (!ab)
>                 return;
>         audit_log_format(ab, "auid=%u ses=%u op=%s",
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index bf309f2..26a80a9 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -1091,7 +1091,7 @@ static void audit_log_rule_change(char *action, struct audit_krule *rule, int re
>         if (!audit_enabled)
>                 return;
>
> -       ab = audit_log_start(NULL, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
> +       ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_CONFIG_CHANGE);
>         if (!ab)
>                 return;
>         audit_log_session_info(ab);
> --
> 1.8.3.1
>


--
paul moore
www.paul-moore.com

Re: [PATCH ghak59 V2 3/6] audit: exclude user records from syscall context

[Paul Moore]

On Fri, Jul 27, 2018 at 3:51 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> Since the function audit_log_common_recv_msg() is shared by a number of
> AUDIT_CONFIG_CHANGE and the entire range of AUDIT_USER_* record types,
> and since the AUDIT_CONFIG_CHANGE message type has been converted to a
> syscall accompanied record type, special-case the AUDIT_USER_* range of
> messages so they remain standalone records.
>
> See: https://github.com/linux-audit/audit-kernel/issues/59
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/audit.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

While I'm a big fan of associating records into a single event when
they are related, in the case of these user messages the syscall
information isn't related to the logged user event, it is related with
the process' *logging* of the user event.  I agree with the idea
behind this patch, but I have some comments about the patch itself:

* I understand you separated this out because you weren't sure if it
was desirable, but now that you know it is desirable it really should
be squashed into patch 2/6 so that it is a single change.
* I'm not a fan of creating __audit_log_common_recv_msg() in this
case; let's just create a audit_log_user_recv_msg() (or similar)
instead and leave audit_log_common_recv_msg() as it is after patch
2/6.

> diff --git a/kernel/audit.c b/kernel/audit.c
> index 6cfe3c9..62ada1b 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1054,7 +1054,8 @@ static int audit_netlink_ok(struct sk_buff *skb, u16 msg_type)
>         return err;
>  }
>
> -static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
> +static void __audit_log_common_recv_msg(struct audit_context *context,
> +                                       struct audit_buffer **ab, u16 msg_type)
>  {
>         uid_t uid = from_kuid(&init_user_ns, current_uid());
>         pid_t pid = task_tgid_nr(current);
> @@ -1064,7 +1065,7 @@ static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
>                 return;
>         }
>
> -       *ab = audit_log_start(audit_context(), GFP_KERNEL, msg_type);
> +       *ab = audit_log_start(context, GFP_KERNEL, msg_type);
>         if (unlikely(!*ab))
>                 return;
>         audit_log_format(*ab, "pid=%d uid=%u", pid, uid);
> @@ -1072,6 +1073,11 @@ static void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
>         audit_log_task_context(*ab);
>  }
>
> +static inline void audit_log_common_recv_msg(struct audit_buffer **ab, u16 msg_type)
> +{
> +       __audit_log_common_recv_msg(audit_context(), ab, msg_type);
> +}
> +
>  int is_audit_feature_set(int i)
>  {
>         return af.features & AUDIT_FEATURE_TO_MASK(i);
> @@ -1338,7 +1344,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)
>                                 if (err)
>                                         break;
>                         }
> -                       audit_log_common_recv_msg(&ab, msg_type);
> +                       __audit_log_common_recv_msg(NULL, &ab, msg_type);
>                         if (msg_type != AUDIT_USER_TTY)
>                                 audit_log_format(ab, " msg='%.*s'",
>                                                  AUDIT_MESSAGE_TEXT_MAX,
> --
> 1.8.3.1
>


--
paul moore
www.paul-moore.com

Re: [PATCH ghak59 V2 4/6] audit: hand taken context to audit_kill_trees for syscall logging

[Paul Moore]

On Fri, Jul 27, 2018 at 3:51 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> Since the context is taken from the task in __audit_syscall_exit() and
> __audit_free(), hand it to audit_kill_trees() so it can be used to
> associate with a syscall record.  This requires adding the context
> parameter to kill_rules() rather than using the current audit_context
> (which has been taken).
>
> The callers of trim_marked() and evict_chunk() still have their context.
>
> See: https://github.com/linux-audit/audit-kernel/issues/50
> See: https://github.com/linux-audit/audit-kernel/issues/59
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/audit.h      |  4 ++--
>  kernel/audit_tree.c | 18 ++++++++++--------
>  kernel/auditsc.c    |  4 ++--
>  3 files changed, 14 insertions(+), 12 deletions(-)

This looks okay, but see my comments in 5/6.  Since you're going to
need to respin this anyway, I would suggest rebasing it on to of the
current audit/next as Jan's audit tree changes might cause some merge
fuzz.

> diff --git a/kernel/audit.h b/kernel/audit.h
> index 214e149..f39f7aa 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -312,7 +312,7 @@ extern void audit_log_d_path_exe(struct audit_buffer *ab,
>  extern int audit_tag_tree(char *old, char *new);
>  extern const char *audit_tree_path(struct audit_tree *tree);
>  extern void audit_put_tree(struct audit_tree *tree);
> -extern void audit_kill_trees(struct list_head *list);
> +extern void audit_kill_trees(struct audit_context *context);
>  #else
>  #define audit_remove_tree_rule(rule) BUG()
>  #define audit_add_tree_rule(rule) -EINVAL
> @@ -321,7 +321,7 @@ extern void audit_log_d_path_exe(struct audit_buffer *ab,
>  #define audit_put_tree(tree) (void)0
>  #define audit_tag_tree(old, new) -EINVAL
>  #define audit_tree_path(rule) ""       /* never called */
> -#define audit_kill_trees(list) BUG()
> +#define audit_kill_trees(context) BUG()
>  #endif
>
>  extern char *audit_unpack_string(void **bufp, size_t *remain, size_t len);
> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> index f0b7d30..c2281e3 100644
> --- a/kernel/audit_tree.c
> +++ b/kernel/audit_tree.c
> @@ -493,13 +493,13 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
>         return 0;
>  }
>
> -static void audit_tree_log_remove_rule(struct audit_krule *rule)
> +static void audit_tree_log_remove_rule(struct audit_context *context, struct audit_krule *rule)
>  {
>         struct audit_buffer *ab;
>
>         if (!audit_enabled)
>                 return;
> -       ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_CONFIG_CHANGE);
> +       ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
>         if (unlikely(!ab))
>                 return;
>         audit_log_format(ab, "op=remove_rule");
> @@ -510,7 +510,7 @@ static void audit_tree_log_remove_rule(struct audit_krule *rule)
>         audit_log_end(ab);
>  }
>
> -static void kill_rules(struct audit_tree *tree)
> +static void kill_rules(struct audit_context *context, struct audit_tree *tree)
>  {
>         struct audit_krule *rule, *next;
>         struct audit_entry *entry;
> @@ -521,7 +521,7 @@ static void kill_rules(struct audit_tree *tree)
>                 list_del_init(&rule->rlist);
>                 if (rule->tree) {
>                         /* not a half-baked one */
> -                       audit_tree_log_remove_rule(rule);
> +                       audit_tree_log_remove_rule(context, rule);
>                         if (entry->rule.exe)
>                                 audit_remove_mark(entry->rule.exe);
>                         rule->tree = NULL;
> @@ -584,7 +584,7 @@ static void trim_marked(struct audit_tree *tree)
>                 tree->goner = 1;
>                 spin_unlock(&hash_lock);
>                 mutex_lock(&audit_filter_mutex);
> -               kill_rules(tree);
> +               kill_rules(audit_context(), tree);
>                 list_del_init(&tree->list);
>                 mutex_unlock(&audit_filter_mutex);
>                 prune_one(tree);
> @@ -924,8 +924,10 @@ static void audit_schedule_prune(void)
>   * ... and that one is done if evict_chunk() decides to delay until the end
>   * of syscall.  Runs synchronously.
>   */
> -void audit_kill_trees(struct list_head *list)
> +void audit_kill_trees(struct audit_context *context)
>  {
> +       struct list_head *list = &context->killed_trees;
> +
>         audit_ctl_lock();
>         mutex_lock(&audit_filter_mutex);
>
> @@ -933,7 +935,7 @@ void audit_kill_trees(struct list_head *list)
>                 struct audit_tree *victim;
>
>                 victim = list_entry(list->next, struct audit_tree, list);
> -               kill_rules(victim);
> +               kill_rules(context, victim);
>                 list_del_init(&victim->list);
>
>                 mutex_unlock(&audit_filter_mutex);
> @@ -972,7 +974,7 @@ static void evict_chunk(struct audit_chunk *chunk)
>                 list_del_init(&owner->same_root);
>                 spin_unlock(&hash_lock);
>                 if (!postponed) {
> -                       kill_rules(owner);
> +                       kill_rules(audit_context(), owner);
>                         list_move(&owner->list, &prune_list);
>                         need_prune = 1;
>                 } else {
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index fb20746..986c5ce 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1490,7 +1490,7 @@ void __audit_free(struct task_struct *tsk)
>         if (context->in_syscall && context->current_state == AUDIT_RECORD_CONTEXT)
>                 audit_log_exit(context, tsk);
>         if (!list_empty(&context->killed_trees))
> -               audit_kill_trees(&context->killed_trees);
> +               audit_kill_trees(context);
>
>         audit_free_context(context);
>  }
> @@ -1577,7 +1577,7 @@ void __audit_syscall_exit(int success, long return_code)
>         context->prio = context->state == AUDIT_RECORD_CONTEXT ? ~0ULL : 0;
>
>         if (!list_empty(&context->killed_trees))
> -               audit_kill_trees(&context->killed_trees);
> +               audit_kill_trees(context);
>
>         audit_free_names(context);
>         unroll_tree_refs(context, NULL, 0);
> --
> 1.8.3.1
>


--
paul moore
www.paul-moore.com

Re: [PATCH ghak59 V2 5/6] audit: kill trees before logging syscall exit for exit/free

[Paul Moore]

On Fri, Jul 27, 2018 at 3:51 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> The EOE record was being issued prior to the pruning of the killed_tree
> list.
>
> Move the kill_trees call before the audit_log_exit call in
> __audit_free() and __audit_syscall_exit() so that the user library
> doesn't leave out any appended pruned trees CONFIG_CHANGE records due to
> the EOE flagging the end of the event.
>
> See: https://github.com/linux-audit/audit-kernel/issues/50
> See: https://github.com/linux-audit/audit-kernel/issues/59
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/auditsc.c | 9 ++++-----
>  1 file changed, 4 insertions(+), 5 deletions(-)

This patch and patch 4/6 should be squashed together.

> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 986c5ce..55fd2a3 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1482,6 +1482,8 @@ void __audit_free(struct task_struct *tsk)
>         if (!context)
>                 return;
>
> +       if (!list_empty(&context->killed_trees))
> +               audit_kill_trees(context);
>         /* Check for system calls that do not go through the exit
>          * function (e.g., exit_group), then free context block.
>          * We use GFP_ATOMIC here because we might be doing this
> @@ -1489,8 +1491,6 @@ void __audit_free(struct task_struct *tsk)
>         /* that can happen only if we are called from do_exit() */
>         if (context->in_syscall && context->current_state == AUDIT_RECORD_CONTEXT)
>                 audit_log_exit(context, tsk);
> -       if (!list_empty(&context->killed_trees))
> -               audit_kill_trees(context);
>
>         audit_free_context(context);
>  }
> @@ -1570,15 +1570,14 @@ void __audit_syscall_exit(int success, long return_code)
>         if (!context)
>                 return;
>
> +       if (!list_empty(&context->killed_trees))
> +               audit_kill_trees(context);
>         if (context->in_syscall && context->current_state == AUDIT_RECORD_CONTEXT)
>                 audit_log_exit(context, current);
>
>         context->in_syscall = 0;
>         context->prio = context->state == AUDIT_RECORD_CONTEXT ? ~0ULL : 0;
>
> -       if (!list_empty(&context->killed_trees))
> -               audit_kill_trees(context);
> -
>         audit_free_names(context);
>         unroll_tree_refs(context, NULL, 0);
>         audit_free_aux(context);
> --
> 1.8.3.1
>


--
paul moore
www.paul-moore.com

Re: [PATCH ghak59 V2 6/6] audit: extend config_change mark/watch/tree rule changes

[Paul Moore]

On Fri, Jul 27, 2018 at 3:51 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> Give a clue as to the source of mark, watch and tree rule changes.
>
> See: https://github.com/linux-audit/audit-kernel/issues/50
> See: https://github.com/linux-audit/audit-kernel/issues/59
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  kernel/audit.h          |  4 ++--
>  kernel/audit_fsnotify.c |  2 +-
>  kernel/audit_tree.c     | 24 ++++++++++++------------
>  kernel/audit_watch.c    |  6 ++++--
>  kernel/auditsc.c        |  4 ++--
>  5 files changed, 21 insertions(+), 19 deletions(-)

I'm not really excited about this change right now, let's hold off on
this for now (the formatting isn't great and I'm not sure it is
necessary), we can always add it later.  The record association
improvements should help here too.

> diff --git a/kernel/audit.h b/kernel/audit.h
> index f39f7aa..5e072f5 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -312,7 +312,7 @@ extern void audit_log_d_path_exe(struct audit_buffer *ab,
>  extern int audit_tag_tree(char *old, char *new);
>  extern const char *audit_tree_path(struct audit_tree *tree);
>  extern void audit_put_tree(struct audit_tree *tree);
> -extern void audit_kill_trees(struct audit_context *context);
> +extern void audit_kill_trees(struct audit_context *context, char *trig);
>  #else
>  #define audit_remove_tree_rule(rule) BUG()
>  #define audit_add_tree_rule(rule) -EINVAL
> @@ -321,7 +321,7 @@ extern void audit_log_d_path_exe(struct audit_buffer *ab,
>  #define audit_put_tree(tree) (void)0
>  #define audit_tag_tree(old, new) -EINVAL
>  #define audit_tree_path(rule) ""       /* never called */
> -#define audit_kill_trees(context) BUG()
> +#define audit_kill_trees(context, trig) BUG()
>  #endif
>
>  extern char *audit_unpack_string(void **bufp, size_t *remain, size_t len);
> diff --git a/kernel/audit_fsnotify.c b/kernel/audit_fsnotify.c
> index ae5c89b..d52bb79 100644
> --- a/kernel/audit_fsnotify.c
> +++ b/kernel/audit_fsnotify.c
> @@ -158,7 +158,7 @@ static void audit_autoremove_mark_rule(struct audit_fsnotify_mark *audit_mark)
>         struct audit_krule *rule = audit_mark->rule;
>         struct audit_entry *entry = container_of(rule, struct audit_entry, rule);
>
> -       audit_mark_log_rule_change(audit_mark, "autoremove_rule");
> +       audit_mark_log_rule_change(audit_mark, "autoremove_rule(mark)");
>         audit_del_rule(entry);
>  }
>
> diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
> index c2281e3..2035097 100644
> --- a/kernel/audit_tree.c
> +++ b/kernel/audit_tree.c
> @@ -493,7 +493,7 @@ static int tag_chunk(struct inode *inode, struct audit_tree *tree)
>         return 0;
>  }
>
> -static void audit_tree_log_remove_rule(struct audit_context *context, struct audit_krule *rule)
> +static void audit_tree_log_remove_rule(struct audit_context *context, struct audit_krule *rule, char *trig)
>  {
>         struct audit_buffer *ab;
>
> @@ -502,7 +502,7 @@ static void audit_tree_log_remove_rule(struct audit_context *context, struct aud
>         ab = audit_log_start(context, GFP_KERNEL, AUDIT_CONFIG_CHANGE);
>         if (unlikely(!ab))
>                 return;
> -       audit_log_format(ab, "op=remove_rule");
> +       audit_log_format(ab, "op=remove_rule(tree:%s)", trig);
>         audit_log_format(ab, " dir=");
>         audit_log_untrustedstring(ab, rule->tree->pathname);
>         audit_log_key(ab, rule->filterkey);
> @@ -510,7 +510,7 @@ static void audit_tree_log_remove_rule(struct audit_context *context, struct aud
>         audit_log_end(ab);
>  }
>
> -static void kill_rules(struct audit_context *context, struct audit_tree *tree)
> +static void kill_rules(struct audit_context *context, struct audit_tree *tree, char *trig)
>  {
>         struct audit_krule *rule, *next;
>         struct audit_entry *entry;
> @@ -521,7 +521,7 @@ static void kill_rules(struct audit_context *context, struct audit_tree *tree)
>                 list_del_init(&rule->rlist);
>                 if (rule->tree) {
>                         /* not a half-baked one */
> -                       audit_tree_log_remove_rule(context, rule);
> +                       audit_tree_log_remove_rule(context, rule, trig);
>                         if (entry->rule.exe)
>                                 audit_remove_mark(entry->rule.exe);
>                         rule->tree = NULL;
> @@ -551,7 +551,7 @@ static void prune_one(struct audit_tree *victim)
>
>  /* trim the uncommitted chunks from tree */
>
> -static void trim_marked(struct audit_tree *tree)
> +static void trim_marked(struct audit_tree *tree, char *trig)
>  {
>         struct list_head *p, *q;
>         spin_lock(&hash_lock);
> @@ -584,7 +584,7 @@ static void trim_marked(struct audit_tree *tree)
>                 tree->goner = 1;
>                 spin_unlock(&hash_lock);
>                 mutex_lock(&audit_filter_mutex);
> -               kill_rules(audit_context(), tree);
> +               kill_rules(audit_context(), tree, trig);
>                 list_del_init(&tree->list);
>                 mutex_unlock(&audit_filter_mutex);
>                 prune_one(tree);
> @@ -665,7 +665,7 @@ void audit_trim_trees(void)
>                                 node->index &= ~(1U<<31);
>                 }
>                 spin_unlock(&hash_lock);
> -               trim_marked(tree);
> +               trim_marked(tree, "trim");
>                 drop_collected_mounts(root_mnt);
>  skip_it:
>                 put_tree(tree);
> @@ -798,7 +798,7 @@ int audit_add_tree_rule(struct audit_krule *rule)
>                         node->index &= ~(1U<<31);
>                 spin_unlock(&hash_lock);
>         } else {
> -               trim_marked(tree);
> +               trim_marked(tree, "add");
>                 goto Err;
>         }
>
> @@ -900,7 +900,7 @@ int audit_tag_tree(char *old, char *new)
>                                 node->index &= ~(1U<<31);
>                         spin_unlock(&hash_lock);
>                 } else {
> -                       trim_marked(tree);
> +                       trim_marked(tree, "equiv");
>                 }
>
>                 put_tree(tree);
> @@ -924,7 +924,7 @@ static void audit_schedule_prune(void)
>   * ... and that one is done if evict_chunk() decides to delay until the end
>   * of syscall.  Runs synchronously.
>   */
> -void audit_kill_trees(struct audit_context *context)
> +void audit_kill_trees(struct audit_context *context, char *trig)
>  {
>         struct list_head *list = &context->killed_trees;
>
> @@ -935,7 +935,7 @@ void audit_kill_trees(struct audit_context *context)
>                 struct audit_tree *victim;
>
>                 victim = list_entry(list->next, struct audit_tree, list);
> -               kill_rules(context, victim);
> +               kill_rules(context, victim, trig);
>                 list_del_init(&victim->list);
>
>                 mutex_unlock(&audit_filter_mutex);
> @@ -974,7 +974,7 @@ static void evict_chunk(struct audit_chunk *chunk)
>                 list_del_init(&owner->same_root);
>                 spin_unlock(&hash_lock);
>                 if (!postponed) {
> -                       kill_rules(audit_context(), owner);
> +                       kill_rules(audit_context(), owner, "evict");
>                         list_move(&owner->list, &prune_list);
>                         need_prune = 1;
>                 } else {
> diff --git a/kernel/audit_watch.c b/kernel/audit_watch.c
> index 26c2fae..6f1e060 100644
> --- a/kernel/audit_watch.c
> +++ b/kernel/audit_watch.c
> @@ -317,7 +317,9 @@ static void audit_update_watch(struct audit_parent *parent,
>                         if (oentry->rule.exe)
>                                 audit_remove_mark(oentry->rule.exe);
>
> -                       audit_watch_log_rule_change(r, owatch, "updated_rules");
> +                       audit_watch_log_rule_change(r, owatch, invalidating ?
> +                                                   "updated_rules(watch:inval)" :
> +                                                   "updated_rules(watch:set)");
>
>                         call_rcu(&oentry->rcu, audit_free_rule_rcu);
>                 }
> @@ -345,7 +347,7 @@ static void audit_remove_parent_watches(struct audit_parent *parent)
>         list_for_each_entry_safe(w, nextw, &parent->watches, wlist) {
>                 list_for_each_entry_safe(r, nextr, &w->rules, rlist) {
>                         e = container_of(r, struct audit_entry, rule);
> -                       audit_watch_log_rule_change(r, w, "remove_rule");
> +                       audit_watch_log_rule_change(r, w, "remove_rule(watch:parent)");
>                         if (e->rule.exe)
>                                 audit_remove_mark(e->rule.exe);
>                         list_del(&r->rlist);
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 55fd2a3..bb56c3e 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -1483,7 +1483,7 @@ void __audit_free(struct task_struct *tsk)
>                 return;
>
>         if (!list_empty(&context->killed_trees))
> -               audit_kill_trees(context);
> +               audit_kill_trees(context, "free");
>         /* Check for system calls that do not go through the exit
>          * function (e.g., exit_group), then free context block.
>          * We use GFP_ATOMIC here because we might be doing this
> @@ -1571,7 +1571,7 @@ void __audit_syscall_exit(int success, long return_code)
>                 return;
>
>         if (!list_empty(&context->killed_trees))
> -               audit_kill_trees(context);
> +               audit_kill_trees(context, "exit");
>         if (context->in_syscall && context->current_state == AUDIT_RECORD_CONTEXT)
>                 audit_log_exit(context, current);
>
> --
> 1.8.3.1
>


--
paul moore
www.paul-moore.com