audit: group task params

RFC PATCH Applied Richard Guy Briggs 2 participants 16 messages May 13, 2018 – May 15, 2018

Subsystems (15)

Networking Network drivers netfilter MM core +11 more subsystems

lore

Share & tools

lore

Richard Guy Briggs

author

2018-05-13 01:58 UTC #0 lore raw

v2 cover audit: group task params

Group the audit parameters for each task into one structure. In particular, remove the loginuid and sessionid values and the audit context pointer from the task structure, replacing them with an audit task information structure to contain them. Use access functions to access audit values.

Note: Use static allocation of the audit task information structure initially. Dynamic allocation was considered and attempted, but isn't ready yet. Static allocation has the limitation that future audit task information structure changes would cause a visible change to the rest of the kernel, whereas dynamic allocation would mostly hide any future changes.

The first four access normalization patches could stand alone.

Passes audit-testsuite.

Changelog:
v2
- p2/5: add audit header to init/init_task.c to quiet kbuildbot
- audit_signal_info(): fetch loginuid once
- remove task_struct from audit_context() param list
- remove extra task_struct local vars
- do nothing on request to set audit context when audit is disabled

Richard Guy Briggs (5):
audit: normalize loginuid read access
audit: convert sessionid unset to a macro
audit: use inline function to get audit context
audit: use inline function to set audit context
audit: collect audit task parameters

 MAINTAINERS                          |  2 +-
 include/linux/audit.h                | 28 ++++++++---
 include/linux/audit_task.h           | 31 ++++++++++++
 include/linux/sched.h                |  6 +--
 include/net/xfrm.h                   |  4 +-
 include/uapi/linux/audit.h           |  1 +
 init/init_task.c                     |  8 ++-
 kernel/audit.c                       |  6 +--
 kernel/audit_watch.c                 |  2 +-
 kernel/auditsc.c                     | 97 +++++++++++++++++-------------------
 kernel/fork.c                        |  2 +-
 net/bridge/netfilter/ebtables.c      |  2 +-
 net/core/dev.c                       |  2 +-
 net/netfilter/x_tables.c             |  2 +-
 net/netlabel/netlabel_user.c         |  2 +-
 security/integrity/ima/ima_api.c     |  2 +-
 security/integrity/integrity_audit.c |  2 +-
 security/lsm_audit.c                 |  2 +-
 security/selinux/hooks.c             |  4 +-
 security/selinux/selinuxfs.c         |  6 +--
 security/selinux/ss/services.c       | 12 ++---
 21 files changed, 133 insertions(+), 90 deletions(-)
 create mode 100644 include/linux/audit_task.h

Signature · 1 line

1.8.3.1

Richard Guy Briggs

author

2018-05-13 01:58 UTC #1 lore raw

RFC PATCH v2 1/5 audit: normalize loginuid read access

Recognizing that the loginuid is an internal audit value, use an access function to retrieve the audit loginuid value for the task rather than reaching directly into the task struct to get it.

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

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

kernel/auditsc.c +9 -9

index 479c031..0d4e269 100644
@@ -374,7 +374,7 @@ static int audit_field_compare(struct task_struct *tsk,
 	case AUDIT_COMPARE_EGID_TO_OBJ_GID:
 		return audit_compare_gid(cred->egid, name, f, ctx);
 	case AUDIT_COMPARE_AUID_TO_OBJ_UID:
-		return audit_compare_uid(tsk->loginuid, name, f, ctx);
+		return audit_compare_uid(audit_get_loginuid(tsk), name, f, ctx);
 	case AUDIT_COMPARE_SUID_TO_OBJ_UID:
 		return audit_compare_uid(cred->suid, name, f, ctx);
 	case AUDIT_COMPARE_SGID_TO_OBJ_GID:

@@ -385,7 +385,7 @@ static int audit_field_compare(struct task_struct *tsk,
 		return audit_compare_gid(cred->fsgid, name, f, ctx);
 	/* uid comparisons */
 	case AUDIT_COMPARE_UID_TO_AUID:
-		return audit_uid_comparator(cred->uid, f->op, tsk->loginuid);
+		return audit_uid_comparator(cred->uid, f->op, audit_get_loginuid(tsk));
 	case AUDIT_COMPARE_UID_TO_EUID:
 		return audit_uid_comparator(cred->uid, f->op, cred->euid);
 	case AUDIT_COMPARE_UID_TO_SUID:

@@ -394,11 +394,11 @@ static int audit_field_compare(struct task_struct *tsk,
 		return audit_uid_comparator(cred->uid, f->op, cred->fsuid);
 	/* auid comparisons */
 	case AUDIT_COMPARE_AUID_TO_EUID:
-		return audit_uid_comparator(tsk->loginuid, f->op, cred->euid);
+		return audit_uid_comparator(audit_get_loginuid(tsk), f->op, cred->euid);
 	case AUDIT_COMPARE_AUID_TO_SUID:
-		return audit_uid_comparator(tsk->loginuid, f->op, cred->suid);
+		return audit_uid_comparator(audit_get_loginuid(tsk), f->op, cred->suid);
 	case AUDIT_COMPARE_AUID_TO_FSUID:
-		return audit_uid_comparator(tsk->loginuid, f->op, cred->fsuid);
+		return audit_uid_comparator(audit_get_loginuid(tsk), f->op, cred->fsuid);
 	/* euid comparisons */
 	case AUDIT_COMPARE_EUID_TO_SUID:
 		return audit_uid_comparator(cred->euid, f->op, cred->suid);

@@ -611,7 +611,7 @@ static int audit_filter_rules(struct task_struct *tsk,
 				result = match_tree_refs(ctx, rule->tree);
 			break;
 		case AUDIT_LOGINUID:
-			result = audit_uid_comparator(tsk->loginuid, f->op, f->uid);
+			result = audit_uid_comparator(audit_get_loginuid(tsk), f->op, f->uid);
 			break;
 		case AUDIT_LOGINUID_SET:
 			result = audit_comparator(audit_loginuid_set(tsk), f->op, f->val);

@@ -2281,14 +2281,14 @@ int audit_signal_info(int sig, struct task_struct *t)
 	struct audit_aux_data_pids *axp;
 	struct task_struct *tsk = current;
 	struct audit_context *ctx = tsk->audit_context;
-	kuid_t uid = current_uid(), t_uid = task_uid(t);
+	kuid_t uid = current_uid(), auid, t_uid = task_uid(t);
 
 	if (auditd_test_task(t) &&
 	    (sig == SIGTERM || sig == SIGHUP ||
 	     sig == SIGUSR1 || sig == SIGUSR2)) {
 		audit_sig_pid = task_tgid_nr(tsk);
-		if (uid_valid(tsk->loginuid))
-			audit_sig_uid = tsk->loginuid;
+		if (uid_valid(auid = audit_get_loginuid(tsk)))
+			audit_sig_uid = auid;
 		else
 			audit_sig_uid = uid;
 		security_task_getsecid(tsk, &audit_sig_sid);

Signature · 1 line

1.8.3.1

Paul Moore

maintainer

2018-05-14 19:52 UTC #2 lore raw

Re: [PATCH ghak81 RFC V2 1/5] audit: normalize loginuid read access

Quoted 63 lines · Richard Guy Briggs

Recognizing that the loginuid is an internal audit value, use an access function to retrieve the audit loginuid value for the task rather than reaching directly into the task struct to get it.

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

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

kernel/auditsc.c +7 -7

index 479c031..0d4e269 100644
@@ -374,7 +374,7 @@ static int audit_field_compare(struct task_struct *tsk,
        case AUDIT_COMPARE_EGID_TO_OBJ_GID:
                return audit_compare_gid(cred->egid, name, f, ctx);
        case AUDIT_COMPARE_AUID_TO_OBJ_UID:
-               return audit_compare_uid(tsk->loginuid, name, f, ctx);
+               return audit_compare_uid(audit_get_loginuid(tsk), name, f, ctx);
        case AUDIT_COMPARE_SUID_TO_OBJ_UID:
                return audit_compare_uid(cred->suid, name, f, ctx);
        case AUDIT_COMPARE_SGID_TO_OBJ_GID:

@@ -385,7 +385,7 @@ static int audit_field_compare(struct task_struct *tsk,
                return audit_compare_gid(cred->fsgid, name, f, ctx);
        /* uid comparisons */
        case AUDIT_COMPARE_UID_TO_AUID:
-               return audit_uid_comparator(cred->uid, f->op, tsk->loginuid);
+               return audit_uid_comparator(cred->uid, f->op, audit_get_loginuid(tsk));
        case AUDIT_COMPARE_UID_TO_EUID:
                return audit_uid_comparator(cred->uid, f->op, cred->euid);
        case AUDIT_COMPARE_UID_TO_SUID:

@@ -394,11 +394,11 @@ static int audit_field_compare(struct task_struct *tsk,
                return audit_uid_comparator(cred->uid, f->op, cred->fsuid);
        /* auid comparisons */
        case AUDIT_COMPARE_AUID_TO_EUID:
-               return audit_uid_comparator(tsk->loginuid, f->op, cred->euid);
+               return audit_uid_comparator(audit_get_loginuid(tsk), f->op, cred->euid);
        case AUDIT_COMPARE_AUID_TO_SUID:
-               return audit_uid_comparator(tsk->loginuid, f->op, cred->suid);
+               return audit_uid_comparator(audit_get_loginuid(tsk), f->op, cred->suid);
        case AUDIT_COMPARE_AUID_TO_FSUID:
-               return audit_uid_comparator(tsk->loginuid, f->op, cred->fsuid);
+               return audit_uid_comparator(audit_get_loginuid(tsk), f->op, cred->fsuid);
        /* euid comparisons */
        case AUDIT_COMPARE_EUID_TO_SUID:
                return audit_uid_comparator(cred->euid, f->op, cred->suid);

@@ -611,7 +611,7 @@ static int audit_filter_rules(struct task_struct *tsk,
                                result = match_tree_refs(ctx, rule->tree);
                        break;
                case AUDIT_LOGINUID:
-                       result = audit_uid_comparator(tsk->loginuid, f->op, f->uid);
+                       result = audit_uid_comparator(audit_get_loginuid(tsk), f->op, f->uid);
                        break;
                case AUDIT_LOGINUID_SET:
                        result = audit_comparator(audit_loginuid_set(tsk), f->op, f->val);

@@ -2281,14 +2281,14 @@ int audit_signal_info(int sig, struct task_struct *t)
        struct audit_aux_data_pids *axp;
        struct task_struct *tsk = current;
        struct audit_context *ctx = tsk->audit_context;
-       kuid_t uid = current_uid(), t_uid = task_uid(t);
+       kuid_t uid = current_uid(), auid, t_uid = task_uid(t);
 
        if (auditd_test_task(t) &&

kernel/auditsc.c int audit_signal_info(int sig, struct task_struct *t) · line 2294

Quoted 10 lines

kernel/auditsc.c +2 -2

            (sig == SIGTERM || sig == SIGHUP ||
             sig == SIGUSR1 || sig == SIGUSR2)) {
                audit_sig_pid = task_tgid_nr(tsk);
-               if (uid_valid(tsk->loginuid))
-                       audit_sig_uid = tsk->loginuid;
+               if (uid_valid(auid = audit_get_loginuid(tsk)))
+                       audit_sig_uid = auid;
                else
                        audit_sig_uid = uid;
                security_task_getsecid(tsk, &audit_sig_sid);

A gentle reminder that you should try to make you patches as "checkpatch clean" as possible (see scripts/checkpatch.pl). There are several 80-char warnings, which aren't fatal, but the big no-no is below:

ERROR: do not use assignment in if condition
#72: FILE: kernel/auditsc.c:2290:
+ if (uid_valid(auid = audit_get_loginuid(tsk)))

... while I don't completely agree with everything checkpatch has to say, I definitely agree with checkpatch when it comes to assignments in if conditions.

Signature · 2 lines

paul moore
www.paul-moore.com

Richard Guy Briggs

author

2018-05-14 20:16 UTC #3 lore raw

Re: [PATCH ghak81 RFC V2 1/5] audit: normalize loginuid read access

Quoted 63 lines · Paul Moore

Quoted 62 lines · Richard Guy Briggs

Recognizing that the loginuid is an internal audit value, use an access function to retrieve the audit loginuid value for the task rather than reaching directly into the task struct to get it.

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

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

kernel/auditsc.c +7 -7

index 479c031..0d4e269 100644
@@ -374,7 +374,7 @@ static int audit_field_compare(struct task_struct *tsk,
        case AUDIT_COMPARE_EGID_TO_OBJ_GID:
                return audit_compare_gid(cred->egid, name, f, ctx);
        case AUDIT_COMPARE_AUID_TO_OBJ_UID:
-               return audit_compare_uid(tsk->loginuid, name, f, ctx);
+               return audit_compare_uid(audit_get_loginuid(tsk), name, f, ctx);
        case AUDIT_COMPARE_SUID_TO_OBJ_UID:
                return audit_compare_uid(cred->suid, name, f, ctx);
        case AUDIT_COMPARE_SGID_TO_OBJ_GID:

@@ -385,7 +385,7 @@ static int audit_field_compare(struct task_struct *tsk,
                return audit_compare_gid(cred->fsgid, name, f, ctx);
        /* uid comparisons */
        case AUDIT_COMPARE_UID_TO_AUID:
-               return audit_uid_comparator(cred->uid, f->op, tsk->loginuid);
+               return audit_uid_comparator(cred->uid, f->op, audit_get_loginuid(tsk));
        case AUDIT_COMPARE_UID_TO_EUID:
                return audit_uid_comparator(cred->uid, f->op, cred->euid);
        case AUDIT_COMPARE_UID_TO_SUID:

@@ -394,11 +394,11 @@ static int audit_field_compare(struct task_struct *tsk,
                return audit_uid_comparator(cred->uid, f->op, cred->fsuid);
        /* auid comparisons */
        case AUDIT_COMPARE_AUID_TO_EUID:
-               return audit_uid_comparator(tsk->loginuid, f->op, cred->euid);
+               return audit_uid_comparator(audit_get_loginuid(tsk), f->op, cred->euid);
        case AUDIT_COMPARE_AUID_TO_SUID:
-               return audit_uid_comparator(tsk->loginuid, f->op, cred->suid);
+               return audit_uid_comparator(audit_get_loginuid(tsk), f->op, cred->suid);
        case AUDIT_COMPARE_AUID_TO_FSUID:
-               return audit_uid_comparator(tsk->loginuid, f->op, cred->fsuid);
+               return audit_uid_comparator(audit_get_loginuid(tsk), f->op, cred->fsuid);
        /* euid comparisons */
        case AUDIT_COMPARE_EUID_TO_SUID:
                return audit_uid_comparator(cred->euid, f->op, cred->suid);

@@ -611,7 +611,7 @@ static int audit_filter_rules(struct task_struct *tsk,
                                result = match_tree_refs(ctx, rule->tree);
                        break;
                case AUDIT_LOGINUID:
-                       result = audit_uid_comparator(tsk->loginuid, f->op, f->uid);
+                       result = audit_uid_comparator(audit_get_loginuid(tsk), f->op, f->uid);
                        break;
                case AUDIT_LOGINUID_SET:
                        result = audit_comparator(audit_loginuid_set(tsk), f->op, f->val);

@@ -2281,14 +2281,14 @@ int audit_signal_info(int sig, struct task_struct *t)
        struct audit_aux_data_pids *axp;
        struct task_struct *tsk = current;
        struct audit_context *ctx = tsk->audit_context;
-       kuid_t uid = current_uid(), t_uid = task_uid(t);
+       kuid_t uid = current_uid(), auid, t_uid = task_uid(t);

Quoted 15 lines

kernel/auditsc.c int audit_signal_info(int sig, struct task_struct *t) · line 2294

Quoted 11 lines

kernel/auditsc.c +2 -2

        if (auditd_test_task(t) &&
            (sig == SIGTERM || sig == SIGHUP ||
             sig == SIGUSR1 || sig == SIGUSR2)) {
                audit_sig_pid = task_tgid_nr(tsk);
-               if (uid_valid(tsk->loginuid))
-                       audit_sig_uid = tsk->loginuid;
+               if (uid_valid(auid = audit_get_loginuid(tsk)))
+                       audit_sig_uid = auid;
                else
                        audit_sig_uid = uid;
                security_task_getsecid(tsk, &audit_sig_sid);

A gentle reminder that you should try to make you patches as "checkpatch clean" as possible (see scripts/checkpatch.pl). There are several 80-char warnings, which aren't fatal,

Yeah, a number of the substitutions were already marginally over to start with, so wrapping them would have made the diff harder to read... Some were shorter than the original, but still over.

Quoted 10 lines

but the big no-no is
below:
ERROR: do not use assignment in if condition
#72: FILE: kernel/auditsc.c:2290:
+ if (uid_valid(auid = audit_get_loginuid(tsk)))
... while I don't completely agree with everything checkpatch has to say, I definitely agree with checkpatch when it comes to assignments in if conditions.

I had run it through checkpatch, but obviously not after this seemingly minor last minute edit... While this is correct, I agree it isn't as easy to read or debug.

Quoted 1 line

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

Richard Guy Briggs

author

2018-05-13 01:58 UTC #4 lore raw

RFC PATCH v2 2/5 audit: convert sessionid unset to a macro

In reply to Richard Guy Briggs

Use a macro, "AUDIT_SID_UNSET", to replace each instance of initialization and comparison to an audit session ID.

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

---
 include/linux/audit.h      | 2 +-
 include/net/xfrm.h         | 2 +-
 include/uapi/linux/audit.h | 1 +
 init/init_task.c           | 3 ++-
 kernel/auditsc.c           | 4 ++--
 5 files changed, 7 insertions(+), 5 deletions(-)

include/linux/audit.h +1 -1

index 75d5b03..5f86f7c 100644
@@ -513,7 +513,7 @@ static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
 }
 static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
 {
-	return -1;
+	return AUDIT_SID_UNSET;
 }
 static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
 { }

include/net/xfrm.h +1 -1

index a872379..fcce8ee 100644
@@ -751,7 +751,7 @@ static inline void xfrm_audit_helper_usrinfo(bool task_valid,
 					    audit_get_loginuid(current) :
 					    INVALID_UID);
 	const unsigned int ses = task_valid ? audit_get_sessionid(current) :
-		(unsigned int) -1;
+		AUDIT_SID_UNSET;
 
 	audit_log_format(audit_buf, " auid=%u ses=%u", auid, ses);
 	audit_log_task_context(audit_buf);

include/uapi/linux/audit.h +1 -0

index 4e61a9e..04f9bd2 100644
@@ -465,6 +465,7 @@ struct audit_tty_status {
 };
 
 #define AUDIT_UID_UNSET (unsigned int)-1
+#define AUDIT_SID_UNSET ((unsigned int)-1)
 
 /* audit_rule_data supports filter rules with both integer and string
  * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and

init/init_task.c +2 -1

index 3ac6e75..74f60ba 100644
@@ -9,6 +9,7 @@
 #include <linux/init.h>
 #include <linux/fs.h>
 #include <linux/mm.h>
+#include <linux/audit.h>
 
 #include <asm/pgtable.h>
 #include <linux/uaccess.h>

@@ -119,7 +120,7 @@ struct task_struct init_task
 	.thread_node	= LIST_HEAD_INIT(init_signals.thread_head),
 #ifdef CONFIG_AUDITSYSCALL
 	.loginuid	= INVALID_UID,
-	.sessionid	= (unsigned int)-1,
+	.sessionid	= AUDIT_SID_UNSET,
 #endif
 #ifdef CONFIG_PERF_EVENTS
 	.perf_event_mutex = __MUTEX_INITIALIZER(init_task.perf_event_mutex),

kernel/auditsc.c +2 -2

index 0d4e269..e157595 100644
@@ -2050,7 +2050,7 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
 int audit_set_loginuid(kuid_t loginuid)
 {
 	struct task_struct *task = current;
-	unsigned int oldsessionid, sessionid = (unsigned int)-1;
+	unsigned int oldsessionid, sessionid = AUDIT_SID_UNSET;
 	kuid_t oldloginuid;
 	int rc;

@@ -2064,7 +2064,7 @@ int audit_set_loginuid(kuid_t loginuid)
 	/* are we setting or clearing? */
 	if (uid_valid(loginuid)) {
 		sessionid = (unsigned int)atomic_inc_return(&session_id);
-		if (unlikely(sessionid == (unsigned int)-1))
+		if (unlikely(sessionid == AUDIT_SID_UNSET))
 			sessionid = (unsigned int)atomic_inc_return(&session_id);
 	}

Signature · 1 line

1.8.3.1

Paul Moore

maintainer Applied

2018-05-14 20:15 UTC #5 lore raw

Re: [PATCH ghak81 RFC V2 2/5] audit: convert sessionid unset to a macro

Quoted 11 lines · Richard Guy Briggs

Use a macro, "AUDIT_SID_UNSET", to replace each instance of initialization and comparison to an audit session ID.
Signed-off-by Richard Guy Briggs <[email protected]>
---
 include/linux/audit.h      | 2 +-
 include/net/xfrm.h         | 2 +-
 include/uapi/linux/audit.h | 1 +
 init/init_task.c           | 3 ++-
 kernel/auditsc.c           | 4 ++--
 5 files changed, 7 insertions(+), 5 deletions(-)

Merged, thanks.

Quoted 84 lines

include/linux/audit.h +1 -1

index 75d5b03..5f86f7c 100644
@@ -513,7 +513,7 @@ static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
 }
 static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
 {
-       return -1;
+       return AUDIT_SID_UNSET;
 }
 static inline void audit_ipc_obj(struct kern_ipc_perm *ipcp)
 { }

include/net/xfrm.h +1 -1

index a872379..fcce8ee 100644
@@ -751,7 +751,7 @@ static inline void xfrm_audit_helper_usrinfo(bool task_valid,
                                            audit_get_loginuid(current) :
                                            INVALID_UID);
        const unsigned int ses = task_valid ? audit_get_sessionid(current) :
-               (unsigned int) -1;
+               AUDIT_SID_UNSET;
 
        audit_log_format(audit_buf, " auid=%u ses=%u", auid, ses);
        audit_log_task_context(audit_buf);

include/uapi/linux/audit.h +1 -0

index 4e61a9e..04f9bd2 100644
@@ -465,6 +465,7 @@ struct audit_tty_status {
 };
 
 #define AUDIT_UID_UNSET (unsigned int)-1
+#define AUDIT_SID_UNSET ((unsigned int)-1)
 
 /* audit_rule_data supports filter rules with both integer and string
  * fields.  It corresponds with AUDIT_ADD_RULE, AUDIT_DEL_RULE and

init/init_task.c +2 -1

index 3ac6e75..74f60ba 100644
@@ -9,6 +9,7 @@
 #include <linux/init.h>
 #include <linux/fs.h>
 #include <linux/mm.h>
+#include <linux/audit.h>
 
 #include <asm/pgtable.h>
 #include <linux/uaccess.h>

@@ -119,7 +120,7 @@ struct task_struct init_task
        .thread_node    = LIST_HEAD_INIT(init_signals.thread_head),
 #ifdef CONFIG_AUDITSYSCALL
        .loginuid       = INVALID_UID,
-       .sessionid      = (unsigned int)-1,
+       .sessionid      = AUDIT_SID_UNSET,
 #endif
 #ifdef CONFIG_PERF_EVENTS
        .perf_event_mutex = __MUTEX_INITIALIZER(init_task.perf_event_mutex),

kernel/auditsc.c +2 -3

index 0d4e269..e157595 100644
@@ -2050,7 +2050,7 @@ static void audit_log_set_loginuid(kuid_t koldloginuid, kuid_t kloginuid,
 int audit_set_loginuid(kuid_t loginuid)
 {
        struct task_struct *task = current;
-       unsigned int oldsessionid, sessionid = (unsigned int)-1;
+       unsigned int oldsessionid, sessionid = AUDIT_SID_UNSET;
        kuid_t oldloginuid;
        int rc;

@@ -2064,7 +2064,7 @@ int audit_set_loginuid(kuid_t loginuid)
        /* are we setting or clearing? */
        if (uid_valid(loginuid)) {
                sessionid = (unsigned int)atomic_inc_return(&session_id);
-               if (unlikely(sessionid == (unsigned int)-1))
+               if (unlikely(sessionid == AUDIT_SID_UNSET))
                        sessionid = (unsigned int)atomic_inc_return(&session_id);
        }
 
--

1.8.3.1

Signature · 2 lines

paul moore
www.paul-moore.com

Richard Guy Briggs

author

2018-05-13 01:58 UTC #6 lore raw

RFC PATCH v2 3/5 audit: use inline function to get audit context

In reply to Richard Guy Briggs

Recognizing that the audit context is an internal audit value, use an access function to retrieve the audit context pointer for the task rather than reaching directly into the task struct to get it.

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

---
 include/linux/audit.h                | 14 ++++++--
 include/net/xfrm.h                   |  2 +-
 kernel/audit.c                       |  6 ++--
 kernel/audit_watch.c                 |  2 +-
 kernel/auditsc.c                     | 64 +++++++++++++++++-------------------
 net/bridge/netfilter/ebtables.c      |  2 +-
 net/core/dev.c                       |  2 +-
 net/netfilter/x_tables.c             |  2 +-
 net/netlabel/netlabel_user.c         |  2 +-
 security/integrity/ima/ima_api.c     |  2 +-
 security/integrity/integrity_audit.c |  2 +-
 security/lsm_audit.c                 |  2 +-
 security/selinux/hooks.c             |  4 +--
 security/selinux/selinuxfs.c         |  6 ++--
 security/selinux/ss/services.c       | 12 +++----
 15 files changed, 64 insertions(+), 60 deletions(-)

include/linux/audit.h +11 -3

index 5f86f7c..786aa8e 100644
@@ -235,9 +235,13 @@ extern void __audit_inode_child(struct inode *parent,
 extern void __audit_seccomp(unsigned long syscall, long signr, int code);
 extern void __audit_ptrace(struct task_struct *t);
 
+static inline struct audit_context *audit_context(void)
+{
+	return current->audit_context;
+}
 static inline bool audit_dummy_context(void)
 {
-	void *p = current->audit_context;
+	void *p = audit_context();
 	return !p || *(int *)p;
 }
 static inline void audit_free(struct task_struct *task)

@@ -249,12 +253,12 @@ static inline void audit_syscall_entry(int major, unsigned long a0,
 				       unsigned long a1, unsigned long a2,
 				       unsigned long a3)
 {
-	if (unlikely(current->audit_context))
+	if (unlikely(audit_context()))
 		__audit_syscall_entry(major, a0, a1, a2, a3);
 }
 static inline void audit_syscall_exit(void *pt_regs)
 {
-	if (unlikely(current->audit_context)) {
+	if (unlikely(audit_context())) {
 		int success = is_syscall_success(pt_regs);
 		long return_code = regs_return_value(pt_regs);

@@ -468,6 +472,10 @@ static inline bool audit_dummy_context(void)
 {
 	return true;
 }
+static inline struct audit_context *audit_context(void)
+{
+	return NULL;
+}
 static inline struct filename *audit_reusename(const __user char *name)
 {
 	return NULL;

include/net/xfrm.h +1 -1

index fcce8ee..7f2e31a 100644
@@ -736,7 +736,7 @@ static inline struct audit_buffer *xfrm_audit_start(const char *op)
 
 	if (audit_enabled == 0)
 		return NULL;
-	audit_buf = audit_log_start(current->audit_context, GFP_ATOMIC,
+	audit_buf = audit_log_start(audit_context(), GFP_ATOMIC,
 				    AUDIT_MAC_IPSEC_EVENT);
 	if (audit_buf == NULL)
 		return NULL;

kernel/audit.c +2 -4

index e9f9a90..e7478cb 100644
@@ -1099,8 +1099,7 @@ static void audit_log_feature_change(int which, u32 old_feature, u32 new_feature
 
 	if (audit_enabled == AUDIT_OFF)
 		return;
-	ab = audit_log_start(current->audit_context,
-			     GFP_KERNEL, AUDIT_FEATURE_CHANGE);
+	ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_FEATURE_CHANGE);
 	if (!ab)
 		return;
 	audit_log_task_info(ab, current);

@@ -2317,8 +2316,7 @@ void audit_log_link_denied(const char *operation)
 		return;
 
 	/* Generate AUDIT_ANOM_LINK with subject, operation, outcome. */
-	ab = audit_log_start(current->audit_context, GFP_KERNEL,
-			     AUDIT_ANOM_LINK);
+	ab = audit_log_start(audit_context(), GFP_KERNEL, AUDIT_ANOM_LINK);
 	if (!ab)
 		return;
 	audit_log_format(ab, "op=%s", operation);

kernel/audit_watch.c +1 -1

index 9eb8b35..f1ba889 100644
@@ -274,7 +274,7 @@ static void audit_update_watch(struct audit_parent *parent,
 		/* If the update involves invalidating rules, do the inode-based
 		 * filtering now, so we don't omit records. */
 		if (invalidating && !audit_dummy_context())
-			audit_filter_inodes(current, current->audit_context);
+			audit_filter_inodes(current, audit_context());
 
 		/* updating ino will likely change which audit_hash_list we
 		 * are on so we need a new watch for the new list */

kernel/auditsc.c +31 ~~-33~~

238 lines not on this page · raw message

net/bridge/netfilter/ebtables.c +1 -1

index 032e0fe..894c96a 100644
@@ -1062,7 +1062,7 @@ static int do_replace_finish(struct net *net, struct ebt_replace *repl,
 
 #ifdef CONFIG_AUDIT
 	if (audit_enabled) {
-		audit_log(current->audit_context, GFP_KERNEL,
+		audit_log(audit_context(), GFP_KERNEL,
 			  AUDIT_NETFILTER_CFG,
 			  "table=%s family=%u entries=%u",
 			  repl->name, AF_BRIDGE, repl->nentries);

net/core/dev.c +1 -1

index 969462e..5f38862 100644
@@ -6749,7 +6749,7 @@ static int __dev_set_promiscuity(struct net_device *dev, int inc, bool notify)
 			dev->flags & IFF_PROMISC ? "entered" : "left");
 		if (audit_enabled) {
 			current_uid_gid(&uid, &gid);
-			audit_log(current->audit_context, GFP_ATOMIC,
+			audit_log(audit_context(), GFP_ATOMIC,
 				AUDIT_ANOM_PROMISCUOUS,
 				"dev=%s prom=%d old_prom=%d auid=%u uid=%u gid=%u ses=%u",
 				dev->name, (dev->flags & IFF_PROMISC),

net/netfilter/x_tables.c +1 -1

index 71325fe..1540263 100644
@@ -1414,7 +1414,7 @@ struct xt_table_info *
 
 #ifdef CONFIG_AUDIT
 	if (audit_enabled) {
-		audit_log(current->audit_context, GFP_KERNEL,
+		audit_log(audit_context(), GFP_KERNEL,
 			  AUDIT_NETFILTER_CFG,
 			  "table=%s family=%u entries=%u",
 			  table->name, table->af, private->number);

net/netlabel/netlabel_user.c +1 -1

index 58495f4..2f328af 100644
@@ -104,7 +104,7 @@ struct audit_buffer *netlbl_audit_start_common(int type,
 	if (audit_enabled == 0)
 		return NULL;
 
-	audit_buf = audit_log_start(current->audit_context, GFP_ATOMIC, type);
+	audit_buf = audit_log_start(audit_context(), GFP_ATOMIC, type);
 	if (audit_buf == NULL)
 		return NULL;

security/integrity/ima/ima_api.c +1 -1

index bf88236..a02c5ac 100644
@@ -326,7 +326,7 @@ void ima_audit_measurement(struct integrity_iint_cache *iint,
 		hex_byte_pack(hash + (i * 2), iint->ima_hash->digest[i]);
 	hash[i * 2] = '\0';
 
-	ab = audit_log_start(current->audit_context, GFP_KERNEL,
+	ab = audit_log_start(audit_context(), GFP_KERNEL,
 			     AUDIT_INTEGRITY_RULE);
 	if (!ab)
 		goto out;

security/integrity/integrity_audit.c +1 -1

index 90987d1..ab10a25 100644
@@ -38,7 +38,7 @@ void integrity_audit_msg(int audit_msgno, struct inode *inode,
 	if (!integrity_audit_info && audit_info == 1)	/* Skip info messages */
 		return;
 
-	ab = audit_log_start(current->audit_context, GFP_KERNEL, audit_msgno);
+	ab = audit_log_start(audit_context(), GFP_KERNEL, audit_msgno);
 	audit_log_format(ab, "pid=%d uid=%u auid=%u ses=%u",
 			 task_pid_nr(current),
 			 from_kuid(&init_user_ns, current_cred()->uid),

security/lsm_audit.c +1 -1

index 67703db..f840010 100644
@@ -447,7 +447,7 @@ void common_lsm_audit(struct common_audit_data *a,
 	if (a == NULL)
 		return;
 	/* we use GFP_ATOMIC so we won't sleep */
-	ab = audit_log_start(current->audit_context, GFP_ATOMIC | __GFP_NOWARN,
+	ab = audit_log_start(audit_context(), GFP_ATOMIC | __GFP_NOWARN,
 			     AUDIT_AVC);
 
 	if (ab == NULL)

security/selinux/hooks.c +2 -2

index 4cafe6a..6dd582b 100644
@@ -3294,7 +3294,7 @@ static int selinux_inode_setxattr(struct dentry *dentry, const char *name,
 			} else {
 				audit_size = 0;
 			}
-			ab = audit_log_start(current->audit_context, GFP_ATOMIC, AUDIT_SELINUX_ERR);
+			ab = audit_log_start(audit_context(), GFP_ATOMIC, AUDIT_SELINUX_ERR);
 			audit_log_format(ab, "op=setxattr invalid_context=");
 			audit_log_n_untrustedstring(ab, value, audit_size);
 			audit_log_end(ab);

@@ -6431,7 +6431,7 @@ static int selinux_setprocattr(const char *name, void *value, size_t size)
 					audit_size = size - 1;
 				else
 					audit_size = size;
-				ab = audit_log_start(current->audit_context, GFP_ATOMIC, AUDIT_SELINUX_ERR);
+				ab = audit_log_start(audit_context(), GFP_ATOMIC, AUDIT_SELINUX_ERR);
 				audit_log_format(ab, "op=fscreate invalid_context=");
 				audit_log_n_untrustedstring(ab, value, audit_size);
 				audit_log_end(ab);

security/selinux/selinuxfs.c +3 -3

index efdc633..b4c51d1 100644
@@ -167,7 +167,7 @@ static ssize_t sel_write_enforce(struct file *file, const char __user *buf,
 				      NULL);
 		if (length)
 			goto out;
-		audit_log(current->audit_context, GFP_KERNEL, AUDIT_MAC_STATUS,
+		audit_log(audit_context(), GFP_KERNEL, AUDIT_MAC_STATUS,
 			"enforcing=%d old_enforcing=%d auid=%u ses=%u"
 			" enabled=%d old-enabled=%d lsm=selinux res=1",
 			new_value, old_value,

@@ -303,7 +303,7 @@ static ssize_t sel_write_disable(struct file *file, const char __user *buf,
 		length = selinux_disable(fsi->state);
 		if (length)
 			goto out;
-		audit_log(current->audit_context, GFP_KERNEL, AUDIT_MAC_STATUS,
+		audit_log(audit_context(), GFP_KERNEL, AUDIT_MAC_STATUS,
 			"enforcing=%d old_enforcing=%d auid=%u ses=%u"
 			" enabled=%d old-enabled=%d lsm=selinux res=1",
 			enforcing, enforcing,

@@ -581,7 +581,7 @@ static ssize_t sel_write_load(struct file *file, const char __user *buf,
 	length = count;
 
 out1:
-	audit_log(current->audit_context, GFP_KERNEL, AUDIT_MAC_POLICY_LOAD,
+	audit_log(audit_context(), GFP_KERNEL, AUDIT_MAC_POLICY_LOAD,
 		"auid=%u ses=%u lsm=selinux res=1",
 		from_kuid(&init_user_ns, audit_get_loginuid(current)),
 		audit_get_sessionid(current));

security/selinux/ss/services.c +6 -6

index 8057e19..0fd8ad9 100644
@@ -501,7 +501,7 @@ static void security_dump_masked_av(struct policydb *policydb,
 		goto out;
 
 	/* audit a message */
-	ab = audit_log_start(current->audit_context,
+	ab = audit_log_start(audit_context(),
 			     GFP_ATOMIC, AUDIT_SELINUX_ERR);
 	if (!ab)
 		goto out;

@@ -743,7 +743,7 @@ static int security_validtrans_handle_fail(struct selinux_state *state,
 		goto out;
 	if (context_struct_to_string(p, tcontext, &t, &tlen))
 		goto out;
-	audit_log(current->audit_context, GFP_ATOMIC, AUDIT_SELINUX_ERR,
+	audit_log(audit_context(), GFP_ATOMIC, AUDIT_SELINUX_ERR,
 		  "op=security_validate_transition seresult=denied"
 		  " oldcontext=%s newcontext=%s taskcontext=%s tclass=%s",
 		  o, n, t, sym_name(p, SYM_CLASSES, tclass-1));

@@ -929,7 +929,7 @@ int security_bounded_transition(struct selinux_state *state,
 					      &old_name, &length) &&
 		    !context_struct_to_string(policydb, new_context,
 					      &new_name, &length)) {
-			audit_log(current->audit_context,
+			audit_log(audit_context(),
 				  GFP_ATOMIC, AUDIT_SELINUX_ERR,
 				  "op=security_bounded_transition "
 				  "seresult=denied "

@@ -1586,7 +1586,7 @@ static int compute_sid_handle_invalid_context(
 		goto out;
 	if (context_struct_to_string(policydb, newcontext, &n, &nlen))
 		goto out;
-	audit_log(current->audit_context, GFP_ATOMIC, AUDIT_SELINUX_ERR,
+	audit_log(audit_context(), GFP_ATOMIC, AUDIT_SELINUX_ERR,
 		  "op=security_compute_sid invalid_context=%s"
 		  " scontext=%s"
 		  " tcontext=%s"

@@ -2882,7 +2882,7 @@ int security_set_bools(struct selinux_state *state, int len, int *values)
 
 	for (i = 0; i < len; i++) {
 		if (!!values[i] != policydb->bool_val_to_struct[i]->state) {
-			audit_log(current->audit_context, GFP_ATOMIC,
+			audit_log(audit_context(), GFP_ATOMIC,
 				AUDIT_MAC_CONFIG_CHANGE,
 				"bool=%s val=%d old_val=%d auid=%u ses=%u",
 				sym_name(policydb, SYM_BOOLS, i),

@@ -3025,7 +3025,7 @@ int security_sid_mls_copy(struct selinux_state *state,
 		if (rc) {
 			if (!context_struct_to_string(policydb, &newcon, &s,
 						      &len)) {
-				audit_log(current->audit_context,
+				audit_log(audit_context(),
 					  GFP_ATOMIC, AUDIT_SELINUX_ERR,
 					  "op=security_sid_mls_copy "
 					  "invalid_context=%s", s);

Signature · 1 line

1.8.3.1

Paul Moore

maintainer

2018-05-14 21:44 UTC #7 lore raw

Re: [PATCH ghak81 RFC V2 3/5] audit: use inline function to get audit context

Quoted 14 lines · Richard Guy Briggs

Recognizing that the audit context is an internal audit value, use an access function to retrieve the audit context pointer for the task rather than reaching directly into the task struct to get it.
Signed-off-by Richard Guy Briggs <[email protected]>
---
 include/linux/audit.h                | 14 ++++++--
 include/net/xfrm.h                   |  2 +-
 kernel/audit.c                       |  6 ++--
 kernel/audit_watch.c                 |  2 +-
 kernel/auditsc.c                     | 64 +++++++++++++++++-------------------
 net/bridge/netfilter/ebtables.c      |  2 +-
 net/core/dev.c                       |  2 +-
 net/netfilter/x_tables.c             |  2 +-

Quoted 8 lines

 net/netlabel/netlabel_user.c         |  2 +-
 security/integrity/ima/ima_api.c     |  2 +-
 security/integrity/integrity_audit.c |  2 +-
 security/lsm_audit.c                 |  2 +-
 security/selinux/hooks.c             |  4 +--
 security/selinux/selinuxfs.c         |  6 ++--
 security/selinux/ss/services.c       | 12 +++----
 15 files changed, 64 insertions(+), 60 deletions(-)

Merged, but there was some fuzz due to the missing 1/5 patch and a handfull of checkpatch.pl fixes. Please take a look at the commit in the audit/next branch and if anything looks awry please send a patch to fix it.

Signature · 2 lines

paul moore
www.paul-moore.com

Richard Guy Briggs

author

2018-05-15 03:05 UTC #8 lore raw

Re: [PATCH ghak81 RFC V2 3/5] audit: use inline function to get audit context

Quoted 24 lines · Paul Moore

Quoted 14 lines · Richard Guy Briggs

Recognizing that the audit context is an internal audit value, use an access function to retrieve the audit context pointer for the task rather than reaching directly into the task struct to get it.
Signed-off-by Richard Guy Briggs <[email protected]>
---
 include/linux/audit.h                | 14 ++++++--
 include/net/xfrm.h                   |  2 +-
 kernel/audit.c                       |  6 ++--
 kernel/audit_watch.c                 |  2 +-
 kernel/auditsc.c                     | 64 +++++++++++++++++-------------------
 net/bridge/netfilter/ebtables.c      |  2 +-
 net/core/dev.c                       |  2 +-
 net/netfilter/x_tables.c             |  2 +-

Quoted 8 lines

 net/netlabel/netlabel_user.c         |  2 +-
 security/integrity/ima/ima_api.c     |  2 +-
 security/integrity/integrity_audit.c |  2 +-
 security/lsm_audit.c                 |  2 +-
 security/selinux/hooks.c             |  4 +--
 security/selinux/selinuxfs.c         |  6 ++--
 security/selinux/ss/services.c       | 12 +++----
 15 files changed, 64 insertions(+), 60 deletions(-)

Quoted 4 lines

Merged, but there was some fuzz due to the missing 1/5 patch and a handfull of checkpatch.pl fixes. Please take a look at the commit in the audit/next branch and if anything looks awry please send a patch to fix it.

Some of that fuzz was due to the two patches (ghak46/47) that went through the xelinux tree... There will be a merge conflict.

Otherwise, looks ok.

Quoted 1 line

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

Richard Guy Briggs

author

2018-05-15 03:28 UTC #9 lore raw

Re: [PATCH ghak81 RFC V2 3/5] audit: use inline function to get audit context

Quoted 30 lines · Richard Guy Briggs

Quoted 24 lines · Paul Moore
Quoted 14 lines · Richard Guy Briggs
Recognizing that the audit context is an internal audit value, use an access function to retrieve the audit context pointer for the task rather than reaching directly into the task struct to get it.
Signed-off-by Richard Guy Briggs <[email protected]>
---
 include/linux/audit.h                | 14 ++++++--
 include/net/xfrm.h                   |  2 +-
 kernel/audit.c                       |  6 ++--
 kernel/audit_watch.c                 |  2 +-
 kernel/auditsc.c                     | 64 +++++++++++++++++-------------------
 net/bridge/netfilter/ebtables.c      |  2 +-
 net/core/dev.c                       |  2 +-
 net/netfilter/x_tables.c             |  2 +-
Quoted 8 lines
 net/netlabel/netlabel_user.c         |  2 +-
 security/integrity/ima/ima_api.c     |  2 +-
 security/integrity/integrity_audit.c |  2 +-
 security/lsm_audit.c                 |  2 +-
 security/selinux/hooks.c             |  4 +--
 security/selinux/selinuxfs.c         |  6 ++--
 security/selinux/ss/services.c       | 12 +++----
 15 files changed, 64 insertions(+), 60 deletions(-)
Quoted 4 lines
Merged, but there was some fuzz due to the missing 1/5 patch and a handfull of checkpatch.pl fixes. Please take a look at the commit in the audit/next branch and if anything looks awry please send a patch to fix it.

Quoted 4 lines

Some of that fuzz was due to the two patches (ghak46/47) that went through the xelinux tree... There will be a merge conflict.
Otherwise, looks ok.

Spoke too soon, missed one from the new seccomp actions_logged...

Patch pending...

Quoted 3 lines

Quoted 1 line
paul moore
- RGB

- 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

Richard Guy Briggs

author

2018-05-13 01:58 UTC #10 lore raw

RFC PATCH v2 4/5 audit: use inline function to set audit context

In reply to Richard Guy Briggs

Recognizing that the audit context is an internal audit value, use an access function to set the audit context pointer for the task rather than reaching directly into the task struct to set it.

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

---
 include/linux/audit.h | 6 ++++++
 kernel/auditsc.c      | 7 +++----
 kernel/fork.c         | 2 +-
 3 files changed, 10 insertions(+), 5 deletions(-)

include/linux/audit.h +6 -0

index 786aa8e..f7973e4 100644
@@ -235,6 +235,10 @@ extern void __audit_inode_child(struct inode *parent,
 extern void __audit_seccomp(unsigned long syscall, long signr, int code);
 extern void __audit_ptrace(struct task_struct *t);
 
+static inline void audit_set_context(struct task_struct *task, struct audit_context *ctx)
+{
+	task->audit_context = ctx;
+}
 static inline struct audit_context *audit_context(void)
 {
 	return current->audit_context;

@@ -472,6 +476,8 @@ static inline bool audit_dummy_context(void)
 {
 	return true;
 }
+static inline void audit_set_context(struct task_struct *task, struct audit_context *ctx)
+{ }
 static inline struct audit_context *audit_context(void)
 {
 	return NULL;

kernel/auditsc.c +3 -4

index ecc0c23..d441d68 100644
@@ -865,7 +865,7 @@ static inline struct audit_context *audit_take_context(struct task_struct *tsk,
 		audit_filter_inodes(tsk, context);
 	}
 
-	tsk->audit_context = NULL;
+	audit_set_context(tsk, NULL);
 	return context;
 }

@@ -952,7 +952,7 @@ int audit_alloc(struct task_struct *tsk)
 	}
 	context->filterkey = key;
 
-	tsk->audit_context  = context;
+	audit_set_context(tsk, context);
 	set_tsk_thread_flag(tsk, TIF_SYSCALL_AUDIT);
 	return 0;
 }

@@ -1554,7 +1554,6 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
  */
 void __audit_syscall_exit(int success, long return_code)
 {
-	struct task_struct *tsk = current;
 	struct audit_context *context;
 
 	if (success)

@@ -1589,7 +1588,7 @@ void __audit_syscall_exit(int success, long return_code)
 		kfree(context->filterkey);
 		context->filterkey = NULL;
 	}
-	tsk->audit_context = context;
+	audit_set_context(current, context);
 }
 
 static inline void handle_one(const struct inode *inode)

kernel/fork.c +1 -1

index 242c8c9..cd18448 100644
@@ -1713,7 +1713,7 @@ static __latent_entropy struct task_struct *copy_process(
 	p->start_time = ktime_get_ns();
 	p->real_start_time = ktime_get_boot_ns();
 	p->io_context = NULL;
-	p->audit_context = NULL;
+	audit_set_context(p, NULL);
 	cgroup_fork(p);
 #ifdef CONFIG_NUMA
 	p->mempolicy = mpol_dup(p->mempolicy);

Signature · 1 line

1.8.3.1

Paul Moore

maintainer

2018-05-14 21:51 UTC #11 lore raw

Re: [PATCH ghak81 RFC V2 4/5] audit: use inline function to set audit context

Quoted 10 lines · Richard Guy Briggs

Recognizing that the audit context is an internal audit value, use an access function to set the audit context pointer for the task rather than reaching directly into the task struct to set it.
Signed-off-by Richard Guy Briggs <[email protected]>
---
 include/linux/audit.h | 6 ++++++
 kernel/auditsc.c      | 7 +++----
 kernel/fork.c         | 2 +-
 3 files changed, 10 insertions(+), 5 deletions(-)

Merged with some minor fuzz.

Signature · 2 lines

paul moore
www.paul-moore.com

Richard Guy Briggs

author

2018-05-13 01:58 UTC #12 lore raw

RFC PATCH v2 5/5 audit: collect audit task parameters

In reply to Richard Guy Briggs

The audit-related parameters in struct task_struct should ideally be collected together and accessed through a standard audit API.

Collect the existing loginuid, sessionid and audit_context together in a new struct audit_task_info called "audit" in struct task_struct.

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

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

---
 MAINTAINERS                |  2 +-
 include/linux/audit.h      | 10 +++++-----
 include/linux/audit_task.h | 31 +++++++++++++++++++++++++++++++
 include/linux/sched.h      |  6 ++----
 init/init_task.c           |  7 +++++--
 kernel/auditsc.c           |  6 +++---
 6 files changed, 47 insertions(+), 15 deletions(-)
 create mode 100644 include/linux/audit_task.h

MAINTAINERS +1 -1

index 0a1410d..8c7992d 100644
@@ -2510,7 +2510,7 @@ L:	[email protected] (moderated for non-subscribers)
 W:	https://github.com/linux-audit
 T:	git git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git
 S:	Supported
-F:	include/linux/audit.h
+F:	include/linux/audit*.h
 F:	include/uapi/linux/audit.h
 F:	kernel/audit*

include/linux/audit.h +5 -5

index f7973e4..6d599b6 100644
@@ -237,11 +237,11 @@ extern void __audit_inode_child(struct inode *parent,
 
 static inline void audit_set_context(struct task_struct *task, struct audit_context *ctx)
 {
-	task->audit_context = ctx;
+	task->audit.ctx = ctx;
 }
 static inline struct audit_context *audit_context(void)
 {
-	return current->audit_context;
+	return current->audit.ctx;
 }
 static inline bool audit_dummy_context(void)
 {

@@ -250,7 +250,7 @@ static inline bool audit_dummy_context(void)
 }
 static inline void audit_free(struct task_struct *task)
 {
-	if (unlikely(task->audit_context))
+	if (unlikely(task->audit.ctx))
 		__audit_free(task);
 }
 static inline void audit_syscall_entry(int major, unsigned long a0,

@@ -330,12 +330,12 @@ extern int auditsc_get_stamp(struct audit_context *ctx,
 
 static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
 {
-	return tsk->loginuid;
+	return tsk->audit.loginuid;
 }
 
 static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
 {
-	return tsk->sessionid;
+	return tsk->audit.sessionid;
 }
 
 extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp);

include/linux/audit_task.h new file +31 -0

new file mode 100644
index 0000000..d4b3a20
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* audit_task.h -- definition of audit_task_info structure
+ *
+ * Copyright 2018 Red Hat Inc., Raleigh, North Carolina.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Written by Richard Guy Briggs <[email protected]>
+ *
+ */
+
+#ifndef _LINUX_AUDIT_TASK_H_
+#define _LINUX_AUDIT_TASK_H_
+
+struct audit_context;
+struct audit_task_info {
+	kuid_t			loginuid;
+	unsigned int		sessionid;
+	struct audit_context	*ctx;
+};
+
+#endif

include/linux/sched.h +2 -4

index b3d697f..b58eca0 100644
@@ -27,9 +27,9 @@
 #include <linux/signal_types.h>
 #include <linux/mm_types_task.h>
 #include <linux/task_io_accounting.h>
+#include <linux/audit_task.h>
 
 /* task_struct member predeclarations (sorted alphabetically): */
-struct audit_context;
 struct backing_dev_info;
 struct bio_list;
 struct blk_plug;

@@ -832,10 +832,8 @@ struct task_struct {
 
 	struct callback_head		*task_works;
 
-	struct audit_context		*audit_context;
 #ifdef CONFIG_AUDITSYSCALL
-	kuid_t				loginuid;
-	unsigned int			sessionid;
+	struct audit_task_info		audit;
 #endif
 	struct seccomp			seccomp;

init/init_task.c +5 -2

index 74f60ba..d33260d 100644
@@ -119,8 +119,11 @@ struct task_struct init_task
 	.thread_group	= LIST_HEAD_INIT(init_task.thread_group),
 	.thread_node	= LIST_HEAD_INIT(init_signals.thread_head),
 #ifdef CONFIG_AUDITSYSCALL
-	.loginuid	= INVALID_UID,
-	.sessionid	= AUDIT_SID_UNSET,
+	.audit		= {
+		.loginuid	= INVALID_UID,
+		.sessionid	= AUDIT_SID_UNSET,
+		.ctx		= NULL,
+	},
 #endif
 #ifdef CONFIG_PERF_EVENTS
 	.perf_event_mutex = __MUTEX_INITIALIZER(init_task.perf_event_mutex),

kernel/auditsc.c +3 -3

index d441d68..4c1fd18 100644
@@ -836,7 +836,7 @@ static inline struct audit_context *audit_take_context(struct task_struct *tsk,
 						      int return_valid,
 						      long return_code)
 {
-	struct audit_context *context = tsk->audit_context;
+	struct audit_context *context = tsk->audit.ctx;
 
 	if (!context)
 		return NULL;

@@ -2066,8 +2066,8 @@ int audit_set_loginuid(kuid_t loginuid)
 			sessionid = (unsigned int)atomic_inc_return(&session_id);
 	}
 
-	task->sessionid = sessionid;
-	task->loginuid = loginuid;
+	task->audit.sessionid = sessionid;
+	task->audit.loginuid = loginuid;
 out:
 	audit_log_set_loginuid(oldloginuid, loginuid, oldsessionid, sessionid, rc);
 	return rc;

Signature · 1 line

1.8.3.1

Paul Moore

maintainer

2018-05-14 21:54 UTC #13 lore raw

Re: [PATCH ghak81 RFC V2 5/5] audit: collect audit task parameters

Quoted 9 lines · Richard Guy Briggs

The audit-related parameters in struct task_struct should ideally be collected together and accessed through a standard audit API.
Collect the existing loginuid, sessionid and audit_context together in a new struct audit_task_info called "audit" in struct task_struct.
See: https://github.com/linux-audit/audit-kernel/issues/81
Signed-off-by Richard Guy Briggs <[email protected]>

Quoted 9 lines

---
 MAINTAINERS                |  2 +-
 include/linux/audit.h      | 10 +++++-----
 include/linux/audit_task.h | 31 +++++++++++++++++++++++++++++++
 include/linux/sched.h      |  6 ++----
 init/init_task.c           |  7 +++++--
 kernel/auditsc.c           |  6 +++---
 6 files changed, 47 insertions(+), 15 deletions(-)
 create mode 100644 include/linux/audit_task.h

I'm not going to merge this right now; there is still the question of static vs dynamic (as mentioned in 0/5) and with the main motivation being the audit container ID work, I think it would be good to wait for the next round of those patches before committing to something.

Quoted 164 lines

MAINTAINERS +1 -1

index 0a1410d..8c7992d 100644
@@ -2510,7 +2510,7 @@ L:        [email protected] (moderated for non-subscribers)
 W:     https://github.com/linux-audit
 T:     git git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git
 S:     Supported
-F:     include/linux/audit.h
+F:     include/linux/audit*.h
 F:     include/uapi/linux/audit.h
 F:     kernel/audit*

include/linux/audit.h +5 -5

index f7973e4..6d599b6 100644
@@ -237,11 +237,11 @@ extern void __audit_inode_child(struct inode *parent,
 
 static inline void audit_set_context(struct task_struct *task, struct audit_context *ctx)
 {
-       task->audit_context = ctx;
+       task->audit.ctx = ctx;
 }
 static inline struct audit_context *audit_context(void)
 {
-       return current->audit_context;
+       return current->audit.ctx;
 }
 static inline bool audit_dummy_context(void)
 {

@@ -250,7 +250,7 @@ static inline bool audit_dummy_context(void)
 }
 static inline void audit_free(struct task_struct *task)
 {
-       if (unlikely(task->audit_context))
+       if (unlikely(task->audit.ctx))
                __audit_free(task);
 }
 static inline void audit_syscall_entry(int major, unsigned long a0,

@@ -330,12 +330,12 @@ extern int auditsc_get_stamp(struct audit_context *ctx,
 
 static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
 {
-       return tsk->loginuid;
+       return tsk->audit.loginuid;
 }
 
 static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
 {
-       return tsk->sessionid;
+       return tsk->audit.sessionid;
 }
 
 extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp);

include/linux/audit_task.h new file +31 -0

new file mode 100644
index 0000000..d4b3a20
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* audit_task.h -- definition of audit_task_info structure
+ *
+ * Copyright 2018 Red Hat Inc., Raleigh, North Carolina.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Written by Richard Guy Briggs <[email protected]>
+ *
+ */
+
+#ifndef _LINUX_AUDIT_TASK_H_
+#define _LINUX_AUDIT_TASK_H_
+
+struct audit_context;
+struct audit_task_info {
+       kuid_t                  loginuid;
+       unsigned int            sessionid;
+       struct audit_context    *ctx;
+};
+
+#endif

include/linux/sched.h +2 -4

index b3d697f..b58eca0 100644
@@ -27,9 +27,9 @@
 #include <linux/signal_types.h>
 #include <linux/mm_types_task.h>
 #include <linux/task_io_accounting.h>
+#include <linux/audit_task.h>
 
 /* task_struct member predeclarations (sorted alphabetically): */
-struct audit_context;
 struct backing_dev_info;
 struct bio_list;
 struct blk_plug;

@@ -832,10 +832,8 @@ struct task_struct {
 
        struct callback_head            *task_works;
 
-       struct audit_context            *audit_context;
 #ifdef CONFIG_AUDITSYSCALL
-       kuid_t                          loginuid;
-       unsigned int                    sessionid;
+       struct audit_task_info          audit;
 #endif
        struct seccomp                  seccomp;

init/init_task.c +5 -2

index 74f60ba..d33260d 100644
@@ -119,8 +119,11 @@ struct task_struct init_task
        .thread_group   = LIST_HEAD_INIT(init_task.thread_group),
        .thread_node    = LIST_HEAD_INIT(init_signals.thread_head),
 #ifdef CONFIG_AUDITSYSCALL
-       .loginuid       = INVALID_UID,
-       .sessionid      = AUDIT_SID_UNSET,
+       .audit          = {
+               .loginuid       = INVALID_UID,
+               .sessionid      = AUDIT_SID_UNSET,
+               .ctx            = NULL,
+       },
 #endif
 #ifdef CONFIG_PERF_EVENTS
        .perf_event_mutex = __MUTEX_INITIALIZER(init_task.perf_event_mutex),

kernel/auditsc.c +3 -4

index d441d68..4c1fd18 100644
@@ -836,7 +836,7 @@ static inline struct audit_context *audit_take_context(struct task_struct *tsk,
                                                      int return_valid,
                                                      long return_code)
 {
-       struct audit_context *context = tsk->audit_context;
+       struct audit_context *context = tsk->audit.ctx;
 
        if (!context)
                return NULL;

@@ -2066,8 +2066,8 @@ int audit_set_loginuid(kuid_t loginuid)
                        sessionid = (unsigned int)atomic_inc_return(&session_id);
        }
 
-       task->sessionid = sessionid;
-       task->loginuid = loginuid;
+       task->audit.sessionid = sessionid;
+       task->audit.loginuid = loginuid;
 out:
        audit_log_set_loginuid(oldloginuid, loginuid, oldsessionid, sessionid, rc);
        return rc;
--

1.8.3.1

Signature · 2 lines

paul moore
www.paul-moore.com

Richard Guy Briggs

author

2018-05-15 01:20 UTC #14 lore raw

Re: [PATCH ghak81 RFC V2 5/5] audit: collect audit task parameters

Quoted 20 lines · Paul Moore

Quoted 9 lines · Richard Guy Briggs
The audit-related parameters in struct task_struct should ideally be collected together and accessed through a standard audit API.
Collect the existing loginuid, sessionid and audit_context together in a new struct audit_task_info called "audit" in struct task_struct.
See: https://github.com/linux-audit/audit-kernel/issues/81
Signed-off-by Richard Guy Briggs <[email protected]>
Quoted 9 lines
---
 MAINTAINERS                |  2 +-
 include/linux/audit.h      | 10 +++++-----
 include/linux/audit_task.h | 31 +++++++++++++++++++++++++++++++
 include/linux/sched.h      |  6 ++----
 init/init_task.c           |  7 +++++--
 kernel/auditsc.c           |  6 +++---
 6 files changed, 47 insertions(+), 15 deletions(-)
 create mode 100644 include/linux/audit_task.h

Quoted 4 lines

I'm not going to merge this right now; there is still the question of static vs dynamic (as mentioned in 0/5) and with the main motivation being the audit container ID work, I think it would be good to wait for the next round of those patches before committing to something.

Agreed. I included it for completeness...

Quoted 154 lines

MAINTAINERS +1 -1

index 0a1410d..8c7992d 100644
@@ -2510,7 +2510,7 @@ L:        [email protected] (moderated for non-subscribers)
 W:     https://github.com/linux-audit
 T:     git git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/audit.git
 S:     Supported
-F:     include/linux/audit.h
+F:     include/linux/audit*.h
 F:     include/uapi/linux/audit.h
 F:     kernel/audit*

include/linux/audit.h +5 -5

index f7973e4..6d599b6 100644
@@ -237,11 +237,11 @@ extern void __audit_inode_child(struct inode *parent,
 
 static inline void audit_set_context(struct task_struct *task, struct audit_context *ctx)
 {
-       task->audit_context = ctx;
+       task->audit.ctx = ctx;
 }
 static inline struct audit_context *audit_context(void)
 {
-       return current->audit_context;
+       return current->audit.ctx;
 }
 static inline bool audit_dummy_context(void)
 {

@@ -250,7 +250,7 @@ static inline bool audit_dummy_context(void)
 }
 static inline void audit_free(struct task_struct *task)
 {
-       if (unlikely(task->audit_context))
+       if (unlikely(task->audit.ctx))
                __audit_free(task);
 }
 static inline void audit_syscall_entry(int major, unsigned long a0,

@@ -330,12 +330,12 @@ extern int auditsc_get_stamp(struct audit_context *ctx,
 
 static inline kuid_t audit_get_loginuid(struct task_struct *tsk)
 {
-       return tsk->loginuid;
+       return tsk->audit.loginuid;
 }
 
 static inline unsigned int audit_get_sessionid(struct task_struct *tsk)
 {
-       return tsk->sessionid;
+       return tsk->audit.sessionid;
 }
 
 extern void __audit_ipc_obj(struct kern_ipc_perm *ipcp);

include/linux/audit_task.h new file +31 -0

new file mode 100644
index 0000000..d4b3a20
@@ -0,0 +1,31 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+/* audit_task.h -- definition of audit_task_info structure
+ *
+ * Copyright 2018 Red Hat Inc., Raleigh, North Carolina.
+ * All Rights Reserved.
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * Written by Richard Guy Briggs <[email protected]>
+ *
+ */
+
+#ifndef _LINUX_AUDIT_TASK_H_
+#define _LINUX_AUDIT_TASK_H_
+
+struct audit_context;
+struct audit_task_info {
+       kuid_t                  loginuid;
+       unsigned int            sessionid;
+       struct audit_context    *ctx;
+};
+
+#endif

include/linux/sched.h +2 -4

index b3d697f..b58eca0 100644
@@ -27,9 +27,9 @@
 #include <linux/signal_types.h>
 #include <linux/mm_types_task.h>
 #include <linux/task_io_accounting.h>
+#include <linux/audit_task.h>
 
 /* task_struct member predeclarations (sorted alphabetically): */
-struct audit_context;
 struct backing_dev_info;
 struct bio_list;
 struct blk_plug;

@@ -832,10 +832,8 @@ struct task_struct {
 
        struct callback_head            *task_works;
 
-       struct audit_context            *audit_context;
 #ifdef CONFIG_AUDITSYSCALL
-       kuid_t                          loginuid;
-       unsigned int                    sessionid;
+       struct audit_task_info          audit;
 #endif
        struct seccomp                  seccomp;

init/init_task.c +5 -2

index 74f60ba..d33260d 100644
@@ -119,8 +119,11 @@ struct task_struct init_task
        .thread_group   = LIST_HEAD_INIT(init_task.thread_group),
        .thread_node    = LIST_HEAD_INIT(init_signals.thread_head),
 #ifdef CONFIG_AUDITSYSCALL
-       .loginuid       = INVALID_UID,
-       .sessionid      = AUDIT_SID_UNSET,
+       .audit          = {
+               .loginuid       = INVALID_UID,
+               .sessionid      = AUDIT_SID_UNSET,
+               .ctx            = NULL,
+       },
 #endif
 #ifdef CONFIG_PERF_EVENTS
        .perf_event_mutex = __MUTEX_INITIALIZER(init_task.perf_event_mutex),

kernel/auditsc.c +1 -1

index d441d68..4c1fd18 100644
@@ -836,7 +836,7 @@ static inline struct audit_context *audit_take_context(struct task_struct *tsk,
                                                      int return_valid,
                                                      long return_code)
 {
-       struct audit_context *context = tsk->audit_context;
+       struct audit_context *context = tsk->audit.ctx;
 
        if (!context)
                return NULL;

@@ -2066,8 +2066,8 @@ int audit_set_loginuid(kuid_t loginuid)
                        sessionid = (unsigned int)atomic_inc_return(&session_id);
        }

Quoted 16 lines

Quoted 10 lines

kernel/auditsc.c +2 -3

-       task->sessionid = sessionid;
-       task->loginuid = loginuid;
+       task->audit.sessionid = sessionid;
+       task->audit.loginuid = loginuid;
 out:
        audit_log_set_loginuid(oldloginuid, loginuid, oldsessionid, sessionid, rc);
        return rc;
--

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

maintainer

2018-05-15 01:35 UTC #15 lore raw

Re: [PATCH ghak81 RFC V2 5/5] audit: collect audit task parameters

Quoted 21 lines · Richard Guy Briggs

Quoted 20 lines · Paul Moore
Quoted 18 lines · Richard Guy Briggs
The audit-related parameters in struct task_struct should ideally be collected together and accessed through a standard audit API.
Collect the existing loginuid, sessionid and audit_context together in a new struct audit_task_info called "audit" in struct task_struct.
See: https://github.com/linux-audit/audit-kernel/issues/81
Signed-off-by Richard Guy Briggs <[email protected]>
---
 MAINTAINERS                |  2 +-
 include/linux/audit.h      | 10 +++++-----
 include/linux/audit_task.h | 31 +++++++++++++++++++++++++++++++
 include/linux/sched.h      |  6 ++----
 init/init_task.c           |  7 +++++--
 kernel/auditsc.c           |  6 +++---
 6 files changed, 47 insertions(+), 15 deletions(-)
 create mode 100644 include/linux/audit_task.h

Quoted 6 lines

Quoted 4 lines
I'm not going to merge this right now; there is still the question of static vs dynamic (as mentioned in 0/5) and with the main motivation being the audit container ID work, I think it would be good to wait for the next round of those patches before committing to something.
Agreed. I included it for completeness...

That's what I figured based on the cover letter, and the RFC tag, I just felt a comment making things clear would be helpful.

Signature · 2 lines

paul moore
www.paul-moore.com