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

2018-05-09 Thread Paul Moore 
On Tue, May 8, 2018 at 9:34 PM, Richard Guy Briggs  wrote:
> On 2018-05-04 16:54, Richard Guy Briggs wrote:
>> 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 
>
> There's a minor issue with this patch, adding a header include to
> init/init_task.c in this patch and removing it from patch 5.  That'll be
> in the next revision.

Okay, thanks for the heads-up.  FWIW, this patch looks reasonable in
principle; changing magic numbers to macros/constants is almost always
a step in the right direction.

> I have dynamic allocation working, so that has a good chance of
> appearing  too.

I'll comment on that in your patch 0, I just want to get through the
rest of the patches first.

>> ---
>>  include/linux/audit.h  | 2 +-
>>  include/net/xfrm.h | 2 +-
>>  include/uapi/linux/audit.h | 1 +
>>  init/init_task.c   | 2 +-
>>  kernel/auditsc.c   | 4 ++--
>>  5 files changed, 6 insertions(+), 5 deletions(-)
>>
>> diff --git a/include/linux/audit.h b/include/linux/audit.h
>> index 75d5b03..5f86f7c 100644
>> --- a/include/linux/audit.h
>> +++ b/include/linux/audit.h
>> @@ -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)
>>  { }
>> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
>> index a872379..fcce8ee 100644
>> --- a/include/net/xfrm.h
>> +++ b/include/net/xfrm.h
>> @@ -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);
>> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
>> index 4e61a9e..04f9bd2 100644
>> --- a/include/uapi/linux/audit.h
>> +++ b/include/uapi/linux/audit.h
>> @@ -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
>> diff --git a/init/init_task.c b/init/init_task.c
>> index 3ac6e75..c788f91 100644
>> --- a/init/init_task.c
>> +++ b/init/init_task.c
>> @@ -119,7 +119,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),
>> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
>> index f3817d0..6e3ceb9 100644
>> --- a/kernel/auditsc.c
>> +++ b/kernel/auditsc.c
>> @@ -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(_id);
>> - if (unlikely(sessionid == (unsigned int)-1))
>> + if (unlikely(sessionid == AUDIT_SID_UNSET))
>>   sessionid = (unsigned 
>> int)atomic_inc_return(_id);
>>   }
>>
>> --
>> 1.8.3.1
>>
>> --
>> Linux-audit mailing list
>> linux-au...@redhat.com
>> https://www.redhat.com/mailman/listinfo/linux-audit
>
> - RGB
>
> --
> Richard Guy Briggs 
> 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
> linux-au...@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit



-- 
paul moore
www.paul-moore.com
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2018-05-08 Thread Richard Guy Briggs 
On 2018-05-04 16:54, Richard Guy Briggs wrote:
> 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 

There's a minor issue with this patch, adding a header include to
init/init_task.c in this patch and removing it from patch 5.  That'll be
in the next revision.

I have dynamic allocation working, so that has a good chance of
appearing  too.

> ---
>  include/linux/audit.h  | 2 +-
>  include/net/xfrm.h | 2 +-
>  include/uapi/linux/audit.h | 1 +
>  init/init_task.c   | 2 +-
>  kernel/auditsc.c   | 4 ++--
>  5 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/include/linux/audit.h b/include/linux/audit.h
> index 75d5b03..5f86f7c 100644
> --- a/include/linux/audit.h
> +++ b/include/linux/audit.h
> @@ -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)
>  { }
> diff --git a/include/net/xfrm.h b/include/net/xfrm.h
> index a872379..fcce8ee 100644
> --- a/include/net/xfrm.h
> +++ b/include/net/xfrm.h
> @@ -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);
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 4e61a9e..04f9bd2 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -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
> diff --git a/init/init_task.c b/init/init_task.c
> index 3ac6e75..c788f91 100644
> --- a/init/init_task.c
> +++ b/init/init_task.c
> @@ -119,7 +119,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),
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index f3817d0..6e3ceb9 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -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(_id);
> - if (unlikely(sessionid == (unsigned int)-1))
> + if (unlikely(sessionid == AUDIT_SID_UNSET))
>   sessionid = (unsigned 
> int)atomic_inc_return(_id);
>   }
>  
> -- 
> 1.8.3.1
> 
> --
> Linux-audit mailing list
> linux-au...@redhat.com
> https://www.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs 
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
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

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

2018-05-04 Thread 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 
---
 include/linux/audit.h  | 2 +-
 include/net/xfrm.h | 2 +-
 include/uapi/linux/audit.h | 1 +
 init/init_task.c   | 2 +-
 kernel/auditsc.c   | 4 ++--
 5 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/audit.h b/include/linux/audit.h
index 75d5b03..5f86f7c 100644
--- a/include/linux/audit.h
+++ b/include/linux/audit.h
@@ -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)
 { }
diff --git a/include/net/xfrm.h b/include/net/xfrm.h
index a872379..fcce8ee 100644
--- a/include/net/xfrm.h
+++ b/include/net/xfrm.h
@@ -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);
diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 4e61a9e..04f9bd2 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -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
diff --git a/init/init_task.c b/init/init_task.c
index 3ac6e75..c788f91 100644
--- a/init/init_task.c
+++ b/init/init_task.c
@@ -119,7 +119,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),
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index f3817d0..6e3ceb9 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -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(_id);
-   if (unlikely(sessionid == (unsigned int)-1))
+   if (unlikely(sessionid == AUDIT_SID_UNSET))
sessionid = (unsigned 
int)atomic_inc_return(_id);
}
 
-- 
1.8.3.1

--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html