audit: config_change normalizations and event record gathering
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.
Could I get some feedback on the format of the op field values themselves? They shouldn't cause any text processing headaches but there may be a better way of expressing them.
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
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: move EOE record after kill_trees 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 | 26 ++++++++++++++++++-------- 7 files changed, 57 insertions(+), 35 deletions(-)
Signature · 1 line
1.8.3.1
RFC PATCH 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
--- kernel/audit.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-)
kernel/audit.c +4 -2
@@ -403,7 +403,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)
@@ -1365,7 +1365,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
maintainerRe: [RFC PATCH ghak59 V1 1/6] audit: give a clue what CONFIG_CHANGE op was involved
Quoted 9 lines · Richard Guy Briggs
The failure to add an audit rule due to audit locked gives no clue what
CONFIG_CHANGEoperation failed.
Similarly the set operation is the only other operation that doesn't give the "op=" field to indicate the action.
All otherCONFIG_CHANGErecords include an op= field to give a clue as to what sort of configuration change is being executed.Since these are the only
CONFIG_CHANGErecords that that do not have an op= field, add them to bring them in line with the rest.
Normally this would be an immediate reject because this patch inserts a field into an existing record, but the CONFIG_CHANGE record is so variable (supposedly bad in its own right) that I don't this really matters.
With that out of the way, I think this patch is fine, but I don't think it is complete. At the very least there is another CONFIG_CHANGE record in audit_watch_log_rule_change() that doesn't appear to include an "op" field. If we want to make sure we have an "op" field in every CONFIG_CHANGE record, let's actually add them all :)
There appears to be another one in audit_mark_log_rule_change() ... and one more in audit_receive_msg(). There may be more.
Quoted 41 lines
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=yesNew 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=0type=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
-3@@ -403,7 +403,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)@@ -1365,7 +1365,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
Signature · 2 lines
paul moore www.paul-moore.com
Re: [RFC PATCH ghak59 V1 1/6] audit: give a clue what CONFIG_CHANGE op was involved
Quoted 16 lines · Paul Moore
Quoted 9 lines · Richard Guy Briggs
The failure to add an audit rule due to audit locked gives no clue what
CONFIG_CHANGEoperation failed.
Similarly the set operation is the only other operation that doesn't give the "op=" field to indicate the action.
All otherCONFIG_CHANGErecords include an op= field to give a clue as to what sort of configuration change is being executed.Since these are the only
CONFIG_CHANGErecords that that do not have an op= field, add them to bring them in line with the rest.Normally this would be an immediate reject because this patch inserts a field into an existing record, but the
CONFIG_CHANGErecord is so variable (supposedly bad in its own right) that I don't this really matters.
Quoted 6 lines
With that out of the way, I think this patch is fine, but I don't think it is complete. At the very least there is another
CONFIG_CHANGErecord inaudit_watch_log_rule_change()that doesn't appear to include an "op" field. If we want to make sure we have an "op" field in everyCONFIG_CHANGErecord, let's actually add them all :)
The version I'm looking at already had it when it was added in 2009.
This one doesn't add the auid and ses fields because they will be covered by the linking of this record with the syscall record via the audit_context() introduced in another patch.
Quoted 1 line
There appears to be another one in
audit_mark_log_rule_change()...
Same, 2015.
Quoted 1 line
and one more in
audit_receive_msg(). There may be more.
I believe they're covered by other patches in the ghak59 set.
Quoted 33 lines
Quoted 33 lines
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=yesNew 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=0type=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
-2@@ -403,7 +403,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)@@ -1365,7 +1365,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);
Quoted 12 lines
Quoted 8 lines
kernel/audit.c +3
-1+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
Signature · 2 lines
paul moore www.paul-moore.com
- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore
maintainerRe: [RFC PATCH ghak59 V1 1/6] audit: give a clue what CONFIG_CHANGE op was involved
Quoted 17 lines · Richard Guy Briggs
Quoted 16 lines · Paul Moore
Quoted 9 lines · Richard Guy Briggs
The failure to add an audit rule due to audit locked gives no clue what
CONFIG_CHANGEoperation failed.
Similarly the set operation is the only other operation that doesn't give the "op=" field to indicate the action.
All otherCONFIG_CHANGErecords include an op= field to give a clue as to what sort of configuration change is being executed.Since these are the only
CONFIG_CHANGErecords that that do not have an op= field, add them to bring them in line with the rest.Normally this would be an immediate reject because this patch inserts a field into an existing record, but the
CONFIG_CHANGErecord is so variable (supposedly bad in its own right) that I don't this really matters.
Quoted 8 lines
Quoted 6 lines
With that out of the way, I think this patch is fine, but I don't think it is complete. At the very least there is another
CONFIG_CHANGErecord inaudit_watch_log_rule_change()that doesn't appear to include an "op" field. If we want to make sure we have an "op" field in everyCONFIG_CHANGErecord, let's actually add them all :)The version I'm looking at already had it when it was added in 2009.
Yup, there it is ... now I'm wondering what tree I was looking at as a reference while reviewing this?
/me scratches head
Quoted 3 lines
This one doesn't add the auid and ses fields because they will be covered by the linking of this record with the syscall record via the
audit_context()introduced in another patch.
Yeah, I'm not concerned about that for the reasons you state.
Quoted 3 lines
Quoted 1 line
and one more in
audit_receive_msg(). There may be more.I believe they're covered by other patches in the ghak59 set.
If they are in the later patches it might be good to move those "op=" additions into this patch.
Quoted 54 lines
Quoted 37 lines
Quoted 37 lines
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=yesNew 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=0type=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@@ -403,7 +403,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)@@ -1365,7 +1365,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);Quoted 8 lines
Quoted 4 lines
kernel/audit.c +0
-1return -EPERM ; } --1.8.3.1
--
paul moore
www.paul-moore.com- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Signature · 2 lines
paul moore www.paul-moore.com
Re: [RFC PATCH ghak59 V1 1/6] audit: give a clue what CONFIG_CHANGE op was involved
Quoted 36 lines · Paul Moore
Quoted 17 lines · Richard Guy Briggs
Quoted 16 lines · Paul Moore
Quoted 9 lines · Richard Guy Briggs
The failure to add an audit rule due to audit locked gives no clue what
CONFIG_CHANGEoperation failed.
Similarly the set operation is the only other operation that doesn't give the "op=" field to indicate the action.
All otherCONFIG_CHANGErecords include an op= field to give a clue as to what sort of configuration change is being executed.Since these are the only
CONFIG_CHANGErecords that that do not have an op= field, add them to bring them in line with the rest.Normally this would be an immediate reject because this patch inserts a field into an existing record, but the
CONFIG_CHANGErecord is so variable (supposedly bad in its own right) that I don't this really matters.Quoted 8 lines
Quoted 6 lines
With that out of the way, I think this patch is fine, but I don't think it is complete. At the very least there is another
CONFIG_CHANGErecord inaudit_watch_log_rule_change()that doesn't appear to include an "op" field. If we want to make sure we have an "op" field in everyCONFIG_CHANGErecord, let's actually add them all :)The version I'm looking at already had it when it was added in 2009.
Yup, there it is ... now I'm wondering what tree I was looking at as a reference while reviewing this?
/me scratches head
Quoted 3 lines
This one doesn't add the auid and ses fields because they will be covered by the linking of this record with the syscall record via the
audit_context()introduced in another patch.
Quoted 8 lines
Yeah, I'm not concerned about that for the reasons you state.
Quoted 3 lines
Quoted 1 line
and one more in
audit_receive_msg(). There may be more.I believe they're covered by other patches in the ghak59 set.
If they are in the later patches it might be good to move those "op=" additions into this patch.
I don't see any CONFIG_CHANGE records generated in audit_receive_msg() that are missing op= field. Can you narrow it down?
Quoted 46 lines
Quoted 46 lines
Quoted 45 lines
Quoted 40 lines
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=yesNew 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=0type=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
-3@@ -403,7 +403,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)@@ -1365,7 +1365,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 ; } --Quoted 1 line
1.8.3.1
--
paul moore
www.paul-moore.com
Quoted 14 lines
Quoted 8 lines
- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635Signature · 2 lines
paul moore www.paul-moore.com
- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore
maintainerRe: [RFC PATCH ghak59 V1 1/6] audit: give a clue what CONFIG_CHANGE op was involved
Quoted 41 lines · Richard Guy Briggs
Quoted 40 lines · Paul Moore
Quoted 17 lines · Richard Guy Briggs
Quoted 16 lines · Paul Moore
Quoted 9 lines · Richard Guy Briggs
The failure to add an audit rule due to audit locked gives no clue what
CONFIG_CHANGEoperation failed.
Similarly the set operation is the only other operation that doesn't give the "op=" field to indicate the action.
All otherCONFIG_CHANGErecords include an op= field to give a clue as to what sort of configuration change is being executed.Since these are the only
CONFIG_CHANGErecords that that do not have an op= field, add them to bring them in line with the rest.Normally this would be an immediate reject because this patch inserts a field into an existing record, but the
CONFIG_CHANGErecord is so variable (supposedly bad in its own right) that I don't this really matters.Quoted 8 lines
Quoted 6 lines
With that out of the way, I think this patch is fine, but I don't think it is complete. At the very least there is another
CONFIG_CHANGErecord inaudit_watch_log_rule_change()that doesn't appear to include an "op" field. If we want to make sure we have an "op" field in everyCONFIG_CHANGErecord, let's actually add them all :)The version I'm looking at already had it when it was added in 2009.
Yup, there it is ... now I'm wondering what tree I was looking at as a reference while reviewing this?
/me scratches head
Quoted 3 lines
This one doesn't add the auid and ses fields because they will be covered by the linking of this record with the syscall record via the
audit_context()introduced in another patch.Yeah, I'm not concerned about that for the reasons you state.
Quoted 2 lines
Quoted 1 line
and one more in
audit_receive_msg(). There may be more.
Quoted 7 lines
Quoted 4 lines
Quoted 1 line
I believe they're covered by other patches in the ghak59 set.
If they are in the later patches it might be good to move those "op=" additions into this patch.
I don't see any
CONFIG_CHANGErecords generated inaudit_receive_msg()that are missing op= field. Can you narrow it down?
Well, just grep'ing my way through audit_receive_msg() I see that AUDIT_ADD/DEL_RULE generates a CONFIG_CHANGE record.
Quoted 69 lines
Quoted 57 lines
Quoted 46 lines
Quoted 37 lines
Quoted 37 lines
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=yesNew 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=0type=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@@ -403,7 +403,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)@@ -1365,7 +1365,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);Quoted 8 lines
Quoted 4 lines
kernel/audit.c +0
-1return -EPERM ; } --1.8.3.1
--
paul moore
www.paul-moore.comQuoted 8 lines
- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635Quoted 3 lines
--
paul moore
www.paul-moore.com- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Signature · 2 lines
paul moore www.paul-moore.com
Re: [RFC PATCH ghak59 V1 1/6] audit: give a clue what CONFIG_CHANGE op was involved
Quoted 44 lines · Paul Moore
Quoted 43 lines · Richard Guy Briggs
Quoted 42 lines · Paul Moore
Quoted 17 lines · Richard Guy Briggs
Quoted 16 lines · Paul Moore
Quoted 9 lines · Richard Guy Briggs
The failure to add an audit rule due to audit locked gives no clue what
CONFIG_CHANGEoperation failed.
Similarly the set operation is the only other operation that doesn't give the "op=" field to indicate the action.
All otherCONFIG_CHANGErecords include an op= field to give a clue as to what sort of configuration change is being executed.Since these are the only
CONFIG_CHANGErecords that that do not have an op= field, add them to bring them in line with the rest.Normally this would be an immediate reject because this patch inserts a field into an existing record, but the
CONFIG_CHANGErecord is so variable (supposedly bad in its own right) that I don't this really matters.Quoted 8 lines
Quoted 6 lines
With that out of the way, I think this patch is fine, but I don't think it is complete. At the very least there is another
CONFIG_CHANGErecord inaudit_watch_log_rule_change()that doesn't appear to include an "op" field. If we want to make sure we have an "op" field in everyCONFIG_CHANGErecord, let's actually add them all :)The version I'm looking at already had it when it was added in 2009.
Yup, there it is ... now I'm wondering what tree I was looking at as a reference while reviewing this?
/me scratches head
Quoted 3 lines
This one doesn't add the auid and ses fields because they will be covered by the linking of this record with the syscall record via the
audit_context()introduced in another patch.Yeah, I'm not concerned about that for the reasons you state.
Quoted 3 lines
Quoted 1 line
and one more in
audit_receive_msg(). There may be more.I believe they're covered by other patches in the ghak59 set.
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
If they are in the later patches it might be good to move those "op=" additions into this patch.
I don't see any
CONFIG_CHANGErecords generated inaudit_receive_msg()that are missing op= field. Can you narrow it down?Well, just grep'ing my way through
audit_receive_msg()I see that AUDIT_ADD/DEL_RULE generates aCONFIG_CHANGErecord.
The failure case is addressed in this patch. The success case is addressed in audit_log_rule_change(). The latter already has it. What is the problem? What tree are you looking at? What am I missing?
Quoted 61 lines
Quoted 61 lines
Quoted 60 lines
Quoted 46 lines
Quoted 37 lines
Quoted 37 lines
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=yesNew 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=0type=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@@ -403,7 +403,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)@@ -1365,7 +1365,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);Quoted 8 lines
Quoted 4 lines
kernel/audit.c +0
-1return -EPERM ; } --1.8.3.1
--
paul moore
www.paul-moore.comQuoted 8 lines
- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635--
paul moore
www.paul-moore.com
Quoted 14 lines
Quoted 8 lines
- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635Signature · 2 lines
paul moore www.paul-moore.com
- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore
maintainerRe: [RFC PATCH ghak59 V1 1/6] audit: give a clue what CONFIG_CHANGE op was involved
Quoted 51 lines · Richard Guy Briggs
Quoted 50 lines · Paul Moore
Quoted 48 lines · Richard Guy Briggs
Quoted 36 lines · Paul Moore
Quoted 17 lines · Richard Guy Briggs
Quoted 16 lines · Paul Moore
Quoted 9 lines · Richard Guy Briggs
The failure to add an audit rule due to audit locked gives no clue what
CONFIG_CHANGEoperation failed.
Similarly the set operation is the only other operation that doesn't give the "op=" field to indicate the action.
All otherCONFIG_CHANGErecords include an op= field to give a clue as to what sort of configuration change is being executed.Since these are the only
CONFIG_CHANGErecords that that do not have an op= field, add them to bring them in line with the rest.Normally this would be an immediate reject because this patch inserts a field into an existing record, but the
CONFIG_CHANGErecord is so variable (supposedly bad in its own right) that I don't this really matters.Quoted 8 lines
Quoted 6 lines
With that out of the way, I think this patch is fine, but I don't think it is complete. At the very least there is another
CONFIG_CHANGErecord inaudit_watch_log_rule_change()that doesn't appear to include an "op" field. If we want to make sure we have an "op" field in everyCONFIG_CHANGErecord, let's actually add them all :)The version I'm looking at already had it when it was added in 2009.
Yup, there it is ... now I'm wondering what tree I was looking at as a reference while reviewing this?
/me scratches head
Quoted 3 lines
This one doesn't add the auid and ses fields because they will be covered by the linking of this record with the syscall record via the
audit_context()introduced in another patch.Quoted 8 lines
Yeah, I'm not concerned about that for the reasons you state.
Quoted 3 lines
Quoted 1 line
and one more in
audit_receive_msg(). There may be more.I believe they're covered by other patches in the ghak59 set.
If they are in the later patches it might be good to move those "op=" additions into this patch.
I don't see any
CONFIG_CHANGErecords generated inaudit_receive_msg()that are missing op= field. Can you narrow it down?
Quoted 6 lines
Quoted 2 lines
Well, just grep'ing my way through
audit_receive_msg()I see that AUDIT_ADD/DEL_RULE generates aCONFIG_CHANGErecord.The failure case is addressed in this patch. The success case is addressed in
audit_log_rule_change(). The latter already has it. What is the problem? What tree are you looking at? What am I missing?
So it does. This discussion dragged out long enough that I forgot to check the actual patch submission.
I think this patch is fine, I would recommend updating this patchset using the feedback on the other individual patches and resubmitting.
Signature · 2 lines
paul moore www.paul-moore.com
RFC PATCH 2/6 audit: add syscall information to CONFIG_CHANGE records
In reply to Richard Guy BriggsTie 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
--- 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
@@ -400,7 +400,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);
@@ -1067,7 +1067,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
@@ -127,7 +127,7 @@ static void audit_mark_log_rule_change(struct audit_fsnotify_mark *audit_mark, cif (!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
@@ -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
@@ -242,7 +242,7 @@ static void audit_watch_log_rule_change(struct audit_krule *r, struct audit_watcif (!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
@@ -1093,7 +1093,7 @@ static void audit_log_rule_change(char *action, struct audit_krule *rule, int reif (!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
Re: [RFC PATCH ghak59 V1 2/6] audit: add syscall information to CONFIG_CHANGE records
Quoted 6 lines · Richard Guy Briggs
Tie syscall information to all
CONFIG_CHANGEcalls 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(-)
Merged, thanks.
Quoted 77 lines
kernel/audit.c +2
-2@@ -400,7 +400,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);@@ -1067,7 +1067,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@@ -127,7 +127,7 @@ static void audit_mark_log_rule_change(struct audit_fsnotify_mark *audit_mark, cif (!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@@ -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@@ -242,7 +242,7 @@ static void audit_watch_log_rule_change(struct audit_krule *r, struct audit_watcif (!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@@ -1093,7 +1093,7 @@ static void audit_log_rule_change(char *action, struct audit_krule *rule, int reif (!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
Signature · 2 lines
paul moore www.paul-moore.com
Paul Moore
maintainerRe: [RFC PATCH ghak59 V1 2/6] audit: add syscall information to CONFIG_CHANGE records
Quoted 17 lines · Paul Moore
Quoted 6 lines · Richard Guy Briggs
Tie syscall information to all
CONFIG_CHANGEcalls 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/50Quoted 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(-)
Quoted 1 line
Merged, thanks.
Actually, I take that back (good thing I hadn't pushed yet).
Why don't you squash this patch with 3/6 so that a bisect or cherry-pick backport doesn't end up splitting 2/6 and 3/6 and causing a regression with the AUDIT_USER_* records.
Quoted 82 lines
Quoted 75 lines
kernel/audit.c +2
-2@@ -400,7 +400,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);@@ -1067,7 +1067,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@@ -127,7 +127,7 @@ static void audit_mark_log_rule_change(struct audit_fsnotify_mark *audit_mark, cif (!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@@ -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@@ -242,7 +242,7 @@ static void audit_watch_log_rule_change(struct audit_krule *r, struct audit_watcif (!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@@ -1093,7 +1093,7 @@ static void audit_log_rule_change(char *action, struct audit_krule *rule, int reif (!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
Signature · 2 lines
paul moore www.paul-moore.com
RFC PATCH 3/6 audit: exclude user records from syscall context
In reply to Richard Guy BriggsSince 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
--- kernel/audit.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)
kernel/audit.c +9 -3
@@ -1057,7 +1057,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);
@@ -1067,7 +1068,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);
@@ -1075,6 +1076,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);
@@ -1341,7 +1347,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
maintainerRe: [RFC PATCH ghak59 V1 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 ofAUDIT_CONFIG_CHANGEand the entire range of AUDIT_USER_* record types, and since theAUDIT_CONFIG_CHANGEmessage 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(-)
I think this is fine, but see my previous comment about combining 2/6 and 3/6 as a safety measure.
Quoted 47 lines
kernel/audit.c +9
-4@@ -1057,7 +1057,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);@@ -1067,7 +1068,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);@@ -1075,6 +1076,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);@@ -1341,7 +1347,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
Signature · 2 lines
paul moore www.paul-moore.com
Re: [RFC PATCH ghak59 V1 3/6] audit: exclude user records from syscall context
Quoted 7 lines · Paul Moore
Quoted 6 lines · Richard Guy Briggs
Since the function
audit_log_common_recv_msg()is shared by a number ofAUDIT_CONFIG_CHANGEand the entire range of AUDIT_USER_* record types, and since theAUDIT_CONFIG_CHANGEmessage type has been converted to a syscall accompanied record type, special-case the AUDIT_USER_* range of messages so they remain standalone records.
Quoted 8 lines
Quoted 5 lines
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(-)I think this is fine, but see my previous comment about combining 2/6 and 3/6 as a safety measure.
This one I left as a seperate patch for discussion. We'd previously talked about connecting all possible records with syscall records if they exist, but this one I'm unsure about, since we don't really care what userspace process is issuing this message. It is just the message content itself that is important. Or is it? Are we concerned about CAP_AUDIT_WRITE holders/abusers and want as much info about them as we can get in case they go rogue or pear-shaped?
Quoted 39 lines
Quoted 39 lines
kernel/audit.c +8
-2@@ -1057,7 +1057,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);@@ -1067,7 +1068,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);@@ -1075,6 +1076,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);@@ -1341,7 +1347,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)if (err)break ; }
Quoted 18 lines
Quoted 8 lines
kernel/audit.c +1
-2-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
Signature · 7 lines
paul moore www.paul-moore.com -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Re: [RFC PATCH ghak59 V1 3/6] audit: exclude user records from syscall context
Quoted 17 lines · Richard Guy Briggs
Quoted 7 lines · Paul Moore
Quoted 6 lines · Richard Guy Briggs
Since the function
audit_log_common_recv_msg()is shared by a number ofAUDIT_CONFIG_CHANGEand the entire range of AUDIT_USER_* record types, and since theAUDIT_CONFIG_CHANGEmessage type has been converted to a syscall accompanied record type, special-case the AUDIT_USER_* range of messages so they remain standalone records.Quoted 8 lines
Quoted 5 lines
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(-)I think this is fine, but see my previous comment about combining 2/6 and 3/6 as a safety measure.
Quoted 7 lines
This one I left as a seperate patch for discussion. We'd previously talked about connecting all possible records with syscall records if they exist, but this one I'm unsure about, since we don't really care what userspace process is issuing this message. It is just the message content itself that is important. Or is it? Are we concerned about
CAP_AUDIT_WRITEholders/abusers and want as much info about them as we can get in case they go rogue or pear-shaped?
I'm waiting on re-spinning this patchset because of this open question.
Is connecting AUDIT_USER* records desirable or a liability?
Quoted 67 lines
Quoted 39 lines
Quoted 39 lines
kernel/audit.c +8
-2@@ -1057,7 +1057,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);@@ -1067,7 +1068,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);@@ -1075,6 +1076,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);@@ -1341,7 +1347,7 @@ static int audit_receive_msg(struct sk_buff *skb, struct nlmsghdr *nlh)if (err)break ; }Quoted 18 lines
Quoted 8 lines
kernel/audit.c +1
-2-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
Signature · 7 lines
paul moore www.paul-moore.com -- Linux-audit mailing list [email protected] https://www.redhat.com/mailman/listinfo/linux-audit- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Quoted 4 lines
--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit
- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore
maintainerRe: [RFC PATCH ghak59 V1 3/6] audit: exclude user records from syscall context
Quoted 26 lines · Richard Guy Briggs
Quoted 17 lines · Richard Guy Briggs
Quoted 7 lines · Paul Moore
Quoted 6 lines · Richard Guy Briggs
Since the function
audit_log_common_recv_msg()is shared by a number ofAUDIT_CONFIG_CHANGEand the entire range of AUDIT_USER_* record types, and since theAUDIT_CONFIG_CHANGEmessage type has been converted to a syscall accompanied record type, special-case the AUDIT_USER_* range of messages so they remain standalone records.Quoted 8 lines
Quoted 5 lines
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(-)I think this is fine, but see my previous comment about combining 2/6 and 3/6 as a safety measure.
Quoted 7 lines
This one I left as a seperate patch for discussion. We'd previously talked about connecting all possible records with syscall records if they exist, but this one I'm unsure about, since we don't really care what userspace process is issuing this message. It is just the message content itself that is important. Or is it? Are we concerned about
CAP_AUDIT_WRITEholders/abusers and want as much info about them as we can get in case they go rogue or pear-shaped?
Quoted 3 lines
I'm waiting on re-spinning this patchset because of this open question.
Is connecting AUDIT_USER* records desirable or a liability?
Like I said, I think special casing the AUDIT_USER* records so they are *not* associated with other records is okay, and perhaps even the right thing to do. The problem is that we don't have the necessary context (har har) to match any kernel events (and there is the possibility that there are none) to the userspace generated AUDIT_USER* event. Further, I don't think this is something we would ever be able to solve in a reasonable manner.
Quoted 80 lines
Quoted 67 lines
Quoted 53 lines
Quoted 45 lines
kernel/audit.c +9
-4@@ -1057,7 +1057,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);@@ -1067,7 +1068,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);@@ -1075,6 +1076,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);@@ -1341,7 +1347,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.comQuoted 4 lines
--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635Quoted 4 lines
--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Signature · 2 lines
paul moore www.paul-moore.com
Re: [RFC PATCH ghak59 V1 3/6] audit: exclude user records from syscall context
Quoted 31 lines · Paul Moore
Quoted 26 lines · Richard Guy Briggs
Quoted 17 lines · Richard Guy Briggs
Quoted 7 lines · Paul Moore
Quoted 6 lines · Richard Guy Briggs
Since the function
audit_log_common_recv_msg()is shared by a number ofAUDIT_CONFIG_CHANGEand the entire range of AUDIT_USER_* record types, and since theAUDIT_CONFIG_CHANGEmessage type has been converted to a syscall accompanied record type, special-case the AUDIT_USER_* range of messages so they remain standalone records.Quoted 8 lines
Quoted 5 lines
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(-)I think this is fine, but see my previous comment about combining 2/6 and 3/6 as a safety measure.
Quoted 7 lines
This one I left as a seperate patch for discussion. We'd previously talked about connecting all possible records with syscall records if they exist, but this one I'm unsure about, since we don't really care what userspace process is issuing this message. It is just the message content itself that is important. Or is it? Are we concerned about
CAP_AUDIT_WRITEholders/abusers and want as much info about them as we can get in case they go rogue or pear-shaped?Quoted 3 lines
I'm waiting on re-spinning this patchset because of this open question.
Is connecting AUDIT_USER* records desirable or a liability?
Quoted 7 lines
Like I said, I think special casing the AUDIT_USER* records so they are *not* associated with other records is okay, and perhaps even the right thing to do. The problem is that we don't have the necessary context (har har) to match any kernel events (and there is the possibility that there are none) to the userspace generated AUDIT_USER* event. Further, I don't think this is something we would ever be able to solve in a reasonable manner.
Ok, having said that, I think I'd still prefer to keep this patch seperate, partly to retain the simplicity of the previous patch and make very clear what each one is doing, and partly if we decide to change our mind in the future that these AUDIT_USER* records should be autonomous.
Quoted 72 lines
Quoted 72 lines
Quoted 71 lines
Quoted 53 lines
Quoted 45 lines
kernel/audit.c +9
-4@@ -1057,7 +1057,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);@@ -1067,7 +1068,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);@@ -1075,6 +1076,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);@@ -1341,7 +1347,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.comQuoted 4 lines
--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit
Quoted 14 lines
Quoted 8 lines
- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635Signature · 2 lines
paul moore www.paul-moore.com
- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
Paul Moore
maintainerRe: [RFC PATCH ghak59 V1 3/6] audit: exclude user records from syscall context
Quoted 40 lines · Richard Guy Briggs
Quoted 31 lines · Paul Moore
Quoted 26 lines · Richard Guy Briggs
Quoted 17 lines · Richard Guy Briggs
Quoted 7 lines · Paul Moore
Quoted 6 lines · Richard Guy Briggs
Since the function
audit_log_common_recv_msg()is shared by a number ofAUDIT_CONFIG_CHANGEand the entire range of AUDIT_USER_* record types, and since theAUDIT_CONFIG_CHANGEmessage type has been converted to a syscall accompanied record type, special-case the AUDIT_USER_* range of messages so they remain standalone records.Quoted 8 lines
Quoted 5 lines
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(-)I think this is fine, but see my previous comment about combining 2/6 and 3/6 as a safety measure.
Quoted 7 lines
This one I left as a seperate patch for discussion. We'd previously talked about connecting all possible records with syscall records if they exist, but this one I'm unsure about, since we don't really care what userspace process is issuing this message. It is just the message content itself that is important. Or is it? Are we concerned about
CAP_AUDIT_WRITEholders/abusers and want as much info about them as we can get in case they go rogue or pear-shaped?Quoted 3 lines
I'm waiting on re-spinning this patchset because of this open question.
Is connecting AUDIT_USER* records desirable or a liability?
Quoted 7 lines
Like I said, I think special casing the AUDIT_USER* records so they are *not* associated with other records is okay, and perhaps even the right thing to do. The problem is that we don't have the necessary context (har har) to match any kernel events (and there is the possibility that there are none) to the userspace generated AUDIT_USER* event. Further, I don't think this is something we would ever be able to solve in a reasonable manner.
Quoted 4 lines
Ok, having said that, I think I'd still prefer to keep this patch seperate, partly to retain the simplicity of the previous patch and make very clear what each one is doing, and partly if we decide to change our mind in the future that these AUDIT_USER* records should be autonomous.
Okay, I'll buy that argument.
Signature · 2 lines
paul moore www.paul-moore.com
RFC PATCH 4/6 audit: hand taken context to audit_kill_trees for syscall logging
In reply to Richard Guy BriggsSince 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
--- 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
@@ -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
@@ -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
@@ -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
maintainerRe: [RFC PATCH ghak59 V1 4/6] audit: hand taken context to audit_kill_trees for syscall logging
Quoted 121 lines · Richard Guy Briggs
Since the context is taken from the task in
__audit_syscall_exit()and__audit_free(), hand it toaudit_kill_trees()so it can be used to associate with a syscall record. This requires adding the context parameter tokill_rules()rather than using the currentaudit_context(which has been taken).The callers of
trim_marked()andevict_chunk()still have their context.See: https://github.com/linux-audit/audit-kernel/issues/50
See: https://github.com/linux-audit/audit-kernel/issues/59Signed-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@@ -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@@ -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 +0
-0
kernel/auditsc.c void __audit_free(struct task_struct *tsk) · line 1493
Quoted 5 lines
kernel/auditsc.c +1
-1@@ -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);
See my comment below about the ordering of audit_kill_trees() and audit_log_exit().
kernel/auditsc.c void __audit_syscall_exit(int success, long return_code) · line 1580
Quoted 8 lines
kernel/auditsc.c +1
-1audit_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);
I wonder if we should move the kill_trees if-block above the audit_log_exit() block so that any records that are emitted will be before the SYSCALL record. I didn't chase down all the code paths, but it seems like it should be safe, no?
Quoted 4 lines
kernel/auditsc.c +0
-1audit_free_names (context);unroll_tree_refs (context,NULL ,0 ); --1.8.3.1
Signature · 2 lines
paul moore www.paul-moore.com
Re: [RFC PATCH ghak59 V1 4/6] audit: hand taken context to audit_kill_trees for syscall logging
Quoted 136 lines · Paul Moore
Quoted 121 lines · Richard Guy Briggs
Since the context is taken from the task in
__audit_syscall_exit()and__audit_free(), hand it toaudit_kill_trees()so it can be used to associate with a syscall record. This requires adding the context parameter tokill_rules()rather than using the currentaudit_context(which has been taken).The callers of
trim_marked()andevict_chunk()still have their context.See: https://github.com/linux-audit/audit-kernel/issues/50
See: https://github.com/linux-audit/audit-kernel/issues/59Signed-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@@ -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@@ -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 +0
-0kernel/auditsc.c void __audit_free(struct task_struct *tsk) · line 1493
Quoted 5 lines
kernel/auditsc.c +1
-1@@ -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);See my comment below about the ordering of
audit_kill_trees()andaudit_log_exit().Quoted 5 lines
kernel/auditsc.c +0
-0audit_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 ;
Quoted 8 lines
kernel/auditsc.c void __audit_free(struct task_struct *tsk) · line 1493
Quoted 3 lines
kernel/auditsc.c +1
-1if (!list_empty (&context->killed_trees)) -audit_kill_trees (&context->killed_trees); +audit_kill_trees (context);I wonder if we should move the
kill_treesif-block above theaudit_log_exit()block so that any records that are emitted will be before the SYSCALL record. I didn't chase down all the code paths, but it seems like it should be safe, no?
Interesting. I thought I had looked at re-ordering them and rejected that approach due to that information being needed for audit_log_exit(), but I don't find any such dependency this pass through the code.
I guess the only concern I have then is that if the state is anything other than AUDIT_RECORD_CONTEXT it would be an orphan record, but it would be regardless with the existing code or with my proposed changes. Perhaps that is a bug to start with, though I'm not sure it is at all serious, so I'm not concerned about it.
I think re-ordering should be safe and that eliminates the seeming complexity introduced by the next patch, which is a good thing.
Quoted 4 lines
kernel/auditsc.c void __audit_syscall_exit(int success, long return_code) · line 1583
Quoted 2 lines
kernel/auditsc.c +0
-0audit_free_names (context);unroll_tree_refs (context,NULL ,0 );paul moore
- RGB
--
Richard Guy Briggs <[email protected]>
Sr. S/W Engineer, Kernel Security, Base Operating Systems Remote, Ottawa, Red Hat Canada
IRC: rgb, SunRaycer
Voice: +1.647.777.2635, Internal: (81) 32635
RFC PATCH 5/6 audit: move EOE record after kill_trees for exit/free
In reply to Richard Guy BriggsThe EOE record was being issued prior to the pruning of the killed_tree list.
Move the EOE record creation out of audit_log_exit() and into its callers __audit_free() and __audit_syscall_exit() so that audit_kill_trees() can be called prior to the EOE record creation and any purged trees CONFIG_CHANGE records included in the syscall record event.
See: https://github.com/linux-audit/audit-kernel/issues/50
See: https://github.com/linux-audit/audit-kernel/issues/59
--- kernel/auditsc.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)
kernel/auditsc.c +17 -7
@@ -1460,10 +1460,6 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *tsaudit_log_proctitle (tsk, context); -/* Send end of event record to help user space know we are finished */ - ab =audit_log_start (context,GFP_KERNEL ,AUDIT_EOE ); -if (ab) -audit_log_end (ab);if (call_panic)audit_panic ("error converting sid to string" ); }
@@ -1491,6 +1487,14 @@ void __audit_free(struct task_struct *tsk)audit_log_exit (context, tsk);if (!list_empty (&context->killed_trees))audit_kill_trees (context); +if (context->in_syscall && context->current_state ==AUDIT_RECORD_CONTEXT ) { +struct audit_buffer *ab; + +/* Send end of event record to help user space know we are finished */ + ab =audit_log_start (context,GFP_KERNEL ,AUDIT_EOE ); +if (ab) +audit_log_end (ab); + }audit_free_context (context); }
@@ -1572,13 +1576,19 @@ void __audit_syscall_exit(int success, long return_code)if (context->in_syscall && context->current_state ==AUDIT_RECORD_CONTEXT )audit_log_exit (context, current); +if (!list_empty (&context->killed_trees)) +audit_kill_trees (context); +if (context->in_syscall && context->current_state ==AUDIT_RECORD_CONTEXT ) { +struct audit_buffer *ab; +/* Send end of event record to help user space know we are finished */ + ab =audit_log_start (context,GFP_KERNEL ,AUDIT_EOE ); +if (ab) +audit_log_end (ab); + } 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
maintainerRe: [RFC PATCH ghak59 V1 5/6] audit: move EOE record after kill_trees for exit/free
Quoted 9 lines · Richard Guy Briggs
The EOE record was being issued prior to the pruning of the
killed_treelist.Move the EOE record creation out of
audit_log_exit()and into its callers__audit_free()and__audit_syscall_exit()so thataudit_kill_trees()can be called prior to the EOE record creation and any purged treesCONFIG_CHANGErecords included in the syscall record event.
Quoted 6 lines
See: https://github.com/linux-audit/audit-kernel/issues/50
See: https://github.com/linux-audit/audit-kernel/issues/59Signed-off-by Richard Guy Briggs <[email protected]>--- kernel/auditsc.c | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-)
See my comments in 4/6. Assuming we are able to shuffle the ordering of audit_log_exit() and audit_kill_trees() this patch would no longer be needed, yes?
Quoted 56 lines
kernel/auditsc.c +17
-8@@ -1460,10 +1460,6 @@ static void audit_log_exit(struct audit_context *context, struct task_struct *tsaudit_log_proctitle (tsk, context); -/* Send end of event record to help user space know we are finished */ - ab =audit_log_start (context,GFP_KERNEL ,AUDIT_EOE ); -if (ab) -audit_log_end (ab);if (call_panic)audit_panic ("error converting sid to string" ); }@@ -1491,6 +1487,14 @@ void __audit_free(struct task_struct *tsk)audit_log_exit (context, tsk);if (!list_empty (&context->killed_trees))audit_kill_trees (context); +if (context->in_syscall && context->current_state ==AUDIT_RECORD_CONTEXT ) { +struct audit_buffer *ab; + +/* Send end of event record to help user space know we are finished */ + ab =audit_log_start (context,GFP_KERNEL ,AUDIT_EOE ); +if (ab) +audit_log_end (ab); + }audit_free_context (context); }@@ -1572,13 +1576,19 @@ void __audit_syscall_exit(int success, long return_code)if (context->in_syscall && context->current_state ==AUDIT_RECORD_CONTEXT )audit_log_exit (context, current); +if (!list_empty (&context->killed_trees)) +audit_kill_trees (context); +if (context->in_syscall && context->current_state ==AUDIT_RECORD_CONTEXT ) { +struct audit_buffer *ab; +/* Send end of event record to help user space know we are finished */ + ab =audit_log_start (context,GFP_KERNEL ,AUDIT_EOE ); +if (ab) +audit_log_end (ab); + } 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
Signature · 2 lines
paul moore www.paul-moore.com
RFC PATCH 6/6 audit: extend config_change mark/watch/tree rule changes
In reply to Richard Guy BriggsGive 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
--- 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
@@ -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
@@ -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
@@ -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 audaudit_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
@@ -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
@@ -1486,7 +1486,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); +audit_kill_trees (context,"free" );if (context->in_syscall && context->current_state ==AUDIT_RECORD_CONTEXT ) {struct audit_buffer *ab;
@@ -1577,7 +1577,7 @@ void __audit_syscall_exit(int success, long return_code)if (context->in_syscall && context->current_state ==AUDIT_RECORD_CONTEXT )audit_log_exit (context, current);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 ) {struct audit_buffer *ab;
Signature · 1 line
1.8.3.1
Paul Moore
maintainerRe: [RFC PATCH ghak59 V1 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 think having some additional context here would be helpful for everyone, so I agree with this on principle. However, I think we need to get clarification from Steve that his parser is able to handle these richer "op" values.
Quoted 196 lines
kernel/audit.h +2
-2@@ -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@@ -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@@ -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 audaudit_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@@ -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@@ -1486,7 +1486,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); +audit_kill_trees (context,"free" );if (context->in_syscall && context->current_state ==AUDIT_RECORD_CONTEXT ) {struct audit_buffer *ab;@@ -1577,7 +1577,7 @@ void __audit_syscall_exit(int success, long return_code)if (context->in_syscall && context->current_state ==AUDIT_RECORD_CONTEXT )audit_log_exit (context, current);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 ) {struct audit_buffer *ab; --1.8.3.1
Signature · 2 lines
paul moore www.paul-moore.com
Re: [RFC PATCH ghak59 V1 6/6] audit: extend config_change mark/watch/tree rule changes
Quoted 14 lines · EDT Paul Moore
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/59Quoted 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(-)
Quoted 4 lines
I think having some additional context here would be helpful for everyone, so I agree with this on principle. However, I think we need to get clarification from Steve that his parser is able to handle these richer "op" values.
Op fields are not searchable. So, they normally don't matter. But in general, once they are defined, they should not change. For the record, you can generally insert non-searchable fields anywhere and it doesn't matter. Only the searchable fields like loginuid, uid, pid, exe, etc matter to the parser.
-Steve