audit: config_change normalizations and event record gathering

PATCH Superseded Richard Guy Briggs 2 participants 13 messages Jul 27, 2018 – Nov 19, 2018 Audit

Share & tools

Richard Guy Briggs

author

2018-07-27 19:48 UTC #0 lore raw

v2 cover audit: config_change normalizations and event record gathering

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(-)

Signature · 1 line

1.8.3.1

Richard Guy Briggs

author

2018-07-27 19:48 UTC #1 lore raw

PATCH v2 1/6 audit: give a clue what CONFIG_CHANGE op was involved

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 <[email protected]>

---
 kernel/audit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

kernel/audit.c +4 -2

index 2a80587..1ed8723 100644
@@ -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;
 		}

Signature · 1 line

1.8.3.1

Paul Moore

maintainer

2018-11-19 16:32 UTC #2 lore raw

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

Quoted 39 lines · 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 <[email protected]>
---
 kernel/audit.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
kernel/audit.c +1 -1
index 2a80587..1ed8723 100644
@@ -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)

kernel/audit.c static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) · line 1366

Quoted 6 lines

kernel/audit.c +2 -1

@@ -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".

kernel/audit.c static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh) · line 1370

Quoted 4 lines

kernel/audit.c +1 -0

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

--
paul moore
www.paul-moore.com

Richard Guy Briggs

author

2018-07-27 19:48 UTC #3 lore raw

PATCH v2 2/6 audit: add syscall information to CONFIG_CHANGE records

In reply to 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 <[email protected]>

---
 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(-)

kernel/audit.c +2 -2

index 1ed8723..6cfe3c9 100644
@@ -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);

kernel/audit_fsnotify.c +1 -1

index fba7804..ae5c89b 100644
@@ -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",

kernel/audit_tree.c +1 -1

index 9f6eaeb..f0b7d30 100644
@@ -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");

kernel/audit_watch.c +1 -1

index 787c7af..26c2fae 100644
@@ -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",

kernel/auditfilter.c +1 -1

index bf309f2..26a80a9 100644
@@ -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);

Signature · 1 line

1.8.3.1

Paul Moore

maintainer

2018-11-19 16:32 UTC #4 lore raw

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

Quoted 5 lines · 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

Quoted 8 lines

Signed-off-by Richard Guy Briggs <[email protected]>

---
 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.

Quoted 75 lines

kernel/audit.c +2 -2

index 1ed8723..6cfe3c9 100644
@@ -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);

kernel/audit_fsnotify.c +1 -1

index fba7804..ae5c89b 100644
@@ -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",

kernel/audit_tree.c +1 -1

index 9f6eaeb..f0b7d30 100644
@@ -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");

kernel/audit_watch.c +1 -1

index 787c7af..26c2fae 100644
@@ -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",

kernel/auditfilter.c +1 -2

index bf309f2..26a80a9 100644
@@ -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);
--

Quoted 2 lines

1.8.3.1

--
paul moore
www.paul-moore.com

Richard Guy Briggs

author

2018-07-27 19:48 UTC #5 lore raw

PATCH v2 3/6 audit: exclude user records from syscall context

In reply to 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 <[email protected]>

---
 kernel/audit.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

kernel/audit.c +9 -3

index 6cfe3c9..62ada1b 100644
@@ -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,

Signature · 1 line

1.8.3.1

Paul Moore

maintainer

2018-11-19 16:32 UTC #6 lore raw

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

Quoted 11 lines · 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 <[email protected]>
---
 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.

Quoted 45 lines

kernel/audit.c +9 -4

index 6cfe3c9..62ada1b 100644
@@ -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,
--

Quoted 2 lines

1.8.3.1

--
paul moore
www.paul-moore.com

Richard Guy Briggs

author

2018-07-27 19:48 UTC #7 lore raw

PATCH v2 4/6 audit: hand taken context to audit_kill_trees for syscall logging

In reply to 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 <[email protected]>

---
 kernel/audit.h      |  4 ++--
 kernel/audit_tree.c | 18 ++++++++++--------
 kernel/auditsc.c    |  4 ++--
 3 files changed, 14 insertions(+), 12 deletions(-)

kernel/audit.h +2 -2

index 214e149..f39f7aa 100644
@@ -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);

kernel/audit_tree.c +10 -8

index f0b7d30..c2281e3 100644
@@ -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 {

kernel/auditsc.c +2 -2

index fb20746..986c5ce 100644
@@ -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);

Signature · 1 line

1.8.3.1

Paul Moore

maintainer

2018-11-19 16:32 UTC #8 lore raw

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

Quoted 8 lines · 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.

Quoted 8 lines

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 <[email protected]>
---
 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.

Quoted 122 lines

kernel/audit.h +2 -2

index 214e149..f39f7aa 100644
@@ -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);

kernel/audit_tree.c +10 -8

index f0b7d30..c2281e3 100644
@@ -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 {

kernel/auditsc.c +2 -3

index fb20746..986c5ce 100644
@@ -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);
--

Quoted 2 lines

1.8.3.1

--
paul moore
www.paul-moore.com

Richard Guy Briggs

author

2018-07-27 19:48 UTC #9 lore raw

PATCH v2 5/6 audit: kill trees before logging syscall exit for exit/free

In reply to 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 <[email protected]>

---
 kernel/auditsc.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

kernel/auditsc.c +4 -5

index 986c5ce..55fd2a3 100644
@@ -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);

Signature · 1 line

1.8.3.1

Paul Moore

maintainer

2018-11-19 16:32 UTC #10 lore raw

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

Quoted 8 lines · 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.

Quoted 6 lines

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 <[email protected]>
---
 kernel/auditsc.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

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

Quoted 41 lines

kernel/auditsc.c +4 -6

index 986c5ce..55fd2a3 100644
@@ -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);
--

Quoted 2 lines

1.8.3.1

--
paul moore
www.paul-moore.com

Richard Guy Briggs

author

2018-07-27 19:48 UTC #11 lore raw

PATCH v2 6/6 audit: extend config_change mark/watch/tree rule changes

In reply to 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 <[email protected]>

---
 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(-)

kernel/audit.h +2 -2

index f39f7aa..5e072f5 100644
@@ -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);

kernel/audit_fsnotify.c +1 -1

index ae5c89b..d52bb79 100644
@@ -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);
 }

kernel/audit_tree.c +12 ~~-12~~

index c2281e3..2035097 100644
@@ -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 {

kernel/audit_watch.c +4 -2

index 26c2fae..6f1e060 100644
@@ -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);

kernel/auditsc.c +2 -2

index 55fd2a3..bb56c3e 100644
@@ -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);

Signature · 1 line

1.8.3.1

Paul Moore

maintainer

2018-11-19 16:32 UTC #12 lore raw

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

Quoted 4 lines · 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

Quoted 8 lines

Signed-off-by Richard Guy Briggs <[email protected]>

---
 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.

Quoted 194 lines

kernel/audit.h +2 -2

index f39f7aa..5e072f5 100644
@@ -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);

kernel/audit_fsnotify.c +1 -1

index ae5c89b..d52bb79 100644
@@ -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);
 }

kernel/audit_tree.c +12 ~~-12~~

index c2281e3..2035097 100644
@@ -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 {

kernel/audit_watch.c +4 -2

index 26c2fae..6f1e060 100644
@@ -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);

kernel/auditsc.c +2 -3

index 55fd2a3..bb56c3e 100644
@@ -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);
 
--

Quoted 2 lines

1.8.3.1

--
paul moore
www.paul-moore.com