linux-audit.redhat.com archive mirror
 help / color / mirror / Atom feed
* [PATCH] audit: Rename enum audit_state constants to avoid AUDIT_DISABLED redefinition
@ 2021-06-04 11:21 Sergey Nazarov
  2021-06-04 16:17 ` Richard Guy Briggs
  2021-06-06  2:40 ` Paul Moore
  0 siblings, 2 replies; 10+ messages in thread
From: Sergey Nazarov @ 2021-06-04 11:21 UTC (permalink / raw)
  To: linux-kernel; +Cc: linux-audit, eparis

AUDIT_DISABLED defined in kernel/audit.h as element of enum audit_state
and redefined in kernel/audit.c. This produces a warning when kernel builds
with syscalls audit disabled and brokes kernel build if -Werror used.
enum audit_state used in syscall audit code only. This patch changes
enum audit_state constants prefix AUDIT to AUDITSC to avoid AUDIT_DISABLED
redefinition.

Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
---
 kernel/audit.h   |  8 ++++----
 kernel/auditsc.c | 34 +++++++++++++++++-----------------
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/kernel/audit.h b/kernel/audit.h
index 1522e10..ee81f20 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -21,16 +21,16 @@
    a per-task filter.  At syscall entry, the audit_state is augmented by
    the syscall filter. */
 enum audit_state {
-	AUDIT_DISABLED,		/* Do not create per-task audit_context.
+	AUDITSC_DISABLED,	/* Do not create per-task audit_context.
 				 * No syscall-specific audit records can
 				 * be generated. */
-	AUDIT_BUILD_CONTEXT,	/* Create the per-task audit_context,
+	AUDITSC_BUILD_CONTEXT,	/* Create the per-task audit_context,
 				 * and fill it in at syscall
 				 * entry time.  This makes a full
 				 * syscall record available if some
 				 * other part of the kernel decides it
 				 * should be recorded. */
-	AUDIT_RECORD_CONTEXT	/* Create the per-task audit_context,
+	AUDITSC_RECORD_CONTEXT	/* Create the per-task audit_context,
 				 * always fill it in at syscall entry
 				 * time, and always write out the audit
 				 * record at syscall exit time.  */
@@ -322,7 +322,7 @@ static inline int audit_signal_info_syscall(struct task_struct *t)
 	return 0;
 }
 
-#define audit_filter_inodes(t, c) AUDIT_DISABLED
+#define audit_filter_inodes(t, c) AUDITSC_DISABLED
 #endif /* CONFIG_AUDITSYSCALL */
 
 extern char *audit_unpack_string(void **bufp, size_t *remain, size_t len);
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 175ef6f..ae6e305 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -231,7 +231,7 @@ static void audit_set_auditable(struct audit_context *ctx)
 {
 	if (!ctx->prio) {
 		ctx->prio = 1;
-		ctx->current_state = AUDIT_RECORD_CONTEXT;
+		ctx->current_state = AUDITSC_RECORD_CONTEXT;
 	}
 }
 
@@ -751,10 +751,10 @@ static int audit_filter_rules(struct task_struct *tsk,
 	}
 	switch (rule->action) {
 	case AUDIT_NEVER:
-		*state = AUDIT_DISABLED;
+		*state = AUDITSC_DISABLED;
 		break;
 	case AUDIT_ALWAYS:
-		*state = AUDIT_RECORD_CONTEXT;
+		*state = AUDITSC_RECORD_CONTEXT;
 		break;
 	}
 	return 1;
@@ -773,14 +773,14 @@ static enum audit_state audit_filter_task(struct task_struct *tsk, char **key)
 	list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TASK], list) {
 		if (audit_filter_rules(tsk, &e->rule, NULL, NULL,
 				       &state, true)) {
-			if (state == AUDIT_RECORD_CONTEXT)
+			if (state == AUDITSC_RECORD_CONTEXT)
 				*key = kstrdup(e->rule.filterkey, GFP_ATOMIC);
 			rcu_read_unlock();
 			return state;
 		}
 	}
 	rcu_read_unlock();
-	return AUDIT_BUILD_CONTEXT;
+	return AUDITSC_BUILD_CONTEXT;
 }
 
 static int audit_in_mask(const struct audit_krule *rule, unsigned long val)
@@ -802,7 +802,7 @@ static int audit_in_mask(const struct audit_krule *rule, unsigned long val)
 /* At syscall exit time, this filter is called if the audit_state is
  * not low enough that auditing cannot take place, but is also not
  * high enough that we already know we have to write an audit record
- * (i.e., the state is AUDIT_SETUP_CONTEXT or AUDIT_BUILD_CONTEXT).
+ * (i.e., the state is AUDIT_SETUP_CONTEXT or AUDITSC_BUILD_CONTEXT).
  */
 static void audit_filter_syscall(struct task_struct *tsk,
 				 struct audit_context *ctx)
@@ -923,7 +923,7 @@ static inline struct audit_context *audit_alloc_context(enum audit_state state)
 	if (!context)
 		return NULL;
 	context->state = state;
-	context->prio = state == AUDIT_RECORD_CONTEXT ? ~0ULL : 0;
+	context->prio = state == AUDITSC_RECORD_CONTEXT ? ~0ULL : 0;
 	INIT_LIST_HEAD(&context->killed_trees);
 	INIT_LIST_HEAD(&context->names_list);
 	context->fds[0] = -1;
@@ -950,7 +950,7 @@ int audit_alloc(struct task_struct *tsk)
 		return 0; /* Return if not auditing. */
 
 	state = audit_filter_task(tsk, &key);
-	if (state == AUDIT_DISABLED) {
+	if (state == AUDITSC_DISABLED) {
 		clear_task_syscall_work(tsk, SYSCALL_AUDIT);
 		return 0;
 	}
@@ -1628,7 +1628,7 @@ void __audit_free(struct task_struct *tsk)
 
 		audit_filter_syscall(tsk, context);
 		audit_filter_inodes(tsk, context);
-		if (context->current_state == AUDIT_RECORD_CONTEXT)
+		if (context->current_state == AUDITSC_RECORD_CONTEXT)
 			audit_log_exit();
 	}
 
@@ -1647,7 +1647,7 @@ void __audit_free(struct task_struct *tsk)
  * Fill in audit context at syscall entry.  This only happens if the
  * audit context was created when the task was created and the state or
  * filters demand the audit context be built.  If the state from the
- * per-task filter or from the per-syscall filter is AUDIT_RECORD_CONTEXT,
+ * per-task filter or from the per-syscall filter is AUDITSC_RECORD_CONTEXT,
  * then the record will be written at syscall exit time (otherwise, it
  * will only be written if another part of the kernel requests that it
  * be written).
@@ -1664,11 +1664,11 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
 	BUG_ON(context->in_syscall || context->name_count);
 
 	state = context->state;
-	if (state == AUDIT_DISABLED)
+	if (state == AUDITSC_DISABLED)
 		return;
 
 	context->dummy = !audit_n_rules;
-	if (!context->dummy && state == AUDIT_BUILD_CONTEXT) {
+	if (!context->dummy && state == AUDITSC_BUILD_CONTEXT) {
 		context->prio = 0;
 		if (auditd_test_task(current))
 			return;
@@ -1693,7 +1693,7 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
  * @return_code: return value of the syscall
  *
  * Tear down after system call.  If the audit context has been marked as
- * auditable (either because of the AUDIT_RECORD_CONTEXT state from
+ * auditable (either because of the AUDITSC_RECORD_CONTEXT state from
  * filtering, or because some other part of the kernel wrote an audit
  * message), then write out the syscall information.  In call cases,
  * free the names stored from getname().
@@ -1735,12 +1735,12 @@ void __audit_syscall_exit(int success, long return_code)
 
 		audit_filter_syscall(current, context);
 		audit_filter_inodes(current, context);
-		if (context->current_state == AUDIT_RECORD_CONTEXT)
+		if (context->current_state == AUDITSC_RECORD_CONTEXT)
 			audit_log_exit();
 	}
 
 	context->in_syscall = 0;
-	context->prio = context->state == AUDIT_RECORD_CONTEXT ? ~0ULL : 0;
+	context->prio = context->state == AUDITSC_RECORD_CONTEXT ? ~0ULL : 0;
 
 	audit_free_module(context);
 	audit_free_names(context);
@@ -1753,7 +1753,7 @@ void __audit_syscall_exit(int success, long return_code)
 	context->sockaddr_len = 0;
 	context->type = 0;
 	context->fds[0] = -1;
-	if (context->state != AUDIT_RECORD_CONTEXT) {
+	if (context->state != AUDITSC_RECORD_CONTEXT) {
 		kfree(context->filterkey);
 		context->filterkey = NULL;
 	}
@@ -2203,7 +2203,7 @@ int auditsc_get_stamp(struct audit_context *ctx,
 	*serial    = ctx->serial;
 	if (!ctx->prio) {
 		ctx->prio = 1;
-		ctx->current_state = AUDIT_RECORD_CONTEXT;
+		ctx->current_state = AUDITSC_RECORD_CONTEXT;
 	}
 	return 1;
 }
-- 
1.8.3.1


--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] audit: Rename enum audit_state constants to avoid AUDIT_DISABLED redefinition
  2021-06-04 11:21 [PATCH] audit: Rename enum audit_state constants to avoid AUDIT_DISABLED redefinition Sergey Nazarov
@ 2021-06-04 16:17 ` Richard Guy Briggs
  2021-06-06  2:40 ` Paul Moore
  1 sibling, 0 replies; 10+ messages in thread
From: Richard Guy Briggs @ 2021-06-04 16:17 UTC (permalink / raw)
  To: Sergey Nazarov; +Cc: linux-audit, linux-kernel, eparis

On 2021-06-04 14:21, Sergey Nazarov wrote:
> AUDIT_DISABLED defined in kernel/audit.h as element of enum audit_state
> and redefined in kernel/audit.c. This produces a warning when kernel builds
> with syscalls audit disabled and brokes kernel build if -Werror used.
> enum audit_state used in syscall audit code only. This patch changes
> enum audit_state constants prefix AUDIT to AUDITSC to avoid AUDIT_DISABLED
> redefinition.
> 
> Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>

This is long overdue.  One comment below that needs addressing.

> ---
>  kernel/audit.h   |  8 ++++----
>  kernel/auditsc.c | 34 +++++++++++++++++-----------------
>  2 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/kernel/audit.h b/kernel/audit.h
> index 1522e10..ee81f20 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -21,16 +21,16 @@
>     a per-task filter.  At syscall entry, the audit_state is augmented by
>     the syscall filter. */
>  enum audit_state {
> -	AUDIT_DISABLED,		/* Do not create per-task audit_context.
> +	AUDITSC_DISABLED,	/* Do not create per-task audit_context.
>  				 * No syscall-specific audit records can
>  				 * be generated. */
> -	AUDIT_BUILD_CONTEXT,	/* Create the per-task audit_context,
> +	AUDITSC_BUILD_CONTEXT,	/* Create the per-task audit_context,
>  				 * and fill it in at syscall
>  				 * entry time.  This makes a full
>  				 * syscall record available if some
>  				 * other part of the kernel decides it
>  				 * should be recorded. */
> -	AUDIT_RECORD_CONTEXT	/* Create the per-task audit_context,
> +	AUDITSC_RECORD_CONTEXT	/* Create the per-task audit_context,
>  				 * always fill it in at syscall entry
>  				 * time, and always write out the audit
>  				 * record at syscall exit time.  */
> @@ -322,7 +322,7 @@ static inline int audit_signal_info_syscall(struct task_struct *t)
>  	return 0;
>  }
>  
> -#define audit_filter_inodes(t, c) AUDIT_DISABLED
> +#define audit_filter_inodes(t, c) AUDITSC_DISABLED
>  #endif /* CONFIG_AUDITSYSCALL */
>  
>  extern char *audit_unpack_string(void **bufp, size_t *remain, size_t len);
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 175ef6f..ae6e305 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -231,7 +231,7 @@ static void audit_set_auditable(struct audit_context *ctx)
>  {
>  	if (!ctx->prio) {
>  		ctx->prio = 1;
> -		ctx->current_state = AUDIT_RECORD_CONTEXT;
> +		ctx->current_state = AUDITSC_RECORD_CONTEXT;
>  	}
>  }
>  
> @@ -751,10 +751,10 @@ static int audit_filter_rules(struct task_struct *tsk,
>  	}
>  	switch (rule->action) {
>  	case AUDIT_NEVER:
> -		*state = AUDIT_DISABLED;
> +		*state = AUDITSC_DISABLED;
>  		break;
>  	case AUDIT_ALWAYS:
> -		*state = AUDIT_RECORD_CONTEXT;
> +		*state = AUDITSC_RECORD_CONTEXT;
>  		break;
>  	}
>  	return 1;
> @@ -773,14 +773,14 @@ static enum audit_state audit_filter_task(struct task_struct *tsk, char **key)
>  	list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TASK], list) {
>  		if (audit_filter_rules(tsk, &e->rule, NULL, NULL,
>  				       &state, true)) {
> -			if (state == AUDIT_RECORD_CONTEXT)
> +			if (state == AUDITSC_RECORD_CONTEXT)
>  				*key = kstrdup(e->rule.filterkey, GFP_ATOMIC);
>  			rcu_read_unlock();
>  			return state;
>  		}
>  	}
>  	rcu_read_unlock();
> -	return AUDIT_BUILD_CONTEXT;
> +	return AUDITSC_BUILD_CONTEXT;
>  }
>  
>  static int audit_in_mask(const struct audit_krule *rule, unsigned long val)
> @@ -802,7 +802,7 @@ static int audit_in_mask(const struct audit_krule *rule, unsigned long val)
>  /* At syscall exit time, this filter is called if the audit_state is
>   * not low enough that auditing cannot take place, but is also not
>   * high enough that we already know we have to write an audit record
> - * (i.e., the state is AUDIT_SETUP_CONTEXT or AUDIT_BUILD_CONTEXT).
> + * (i.e., the state is AUDIT_SETUP_CONTEXT or AUDITSC_BUILD_CONTEXT).

That comment should have been updated when AUDIT_SETUP_CONTEXT was
removed on 2012-01-03 in commit 997f5b6444f4608692ec807fb802fd9767c80e76
("audit: remove AUDIT_SETUP_CONTEXT as it isn't used")

>   */
>  static void audit_filter_syscall(struct task_struct *tsk,
>  				 struct audit_context *ctx)
> @@ -923,7 +923,7 @@ static inline struct audit_context *audit_alloc_context(enum audit_state state)
>  	if (!context)
>  		return NULL;
>  	context->state = state;
> -	context->prio = state == AUDIT_RECORD_CONTEXT ? ~0ULL : 0;
> +	context->prio = state == AUDITSC_RECORD_CONTEXT ? ~0ULL : 0;
>  	INIT_LIST_HEAD(&context->killed_trees);
>  	INIT_LIST_HEAD(&context->names_list);
>  	context->fds[0] = -1;
> @@ -950,7 +950,7 @@ int audit_alloc(struct task_struct *tsk)
>  		return 0; /* Return if not auditing. */
>  
>  	state = audit_filter_task(tsk, &key);
> -	if (state == AUDIT_DISABLED) {
> +	if (state == AUDITSC_DISABLED) {
>  		clear_task_syscall_work(tsk, SYSCALL_AUDIT);
>  		return 0;
>  	}
> @@ -1628,7 +1628,7 @@ void __audit_free(struct task_struct *tsk)
>  
>  		audit_filter_syscall(tsk, context);
>  		audit_filter_inodes(tsk, context);
> -		if (context->current_state == AUDIT_RECORD_CONTEXT)
> +		if (context->current_state == AUDITSC_RECORD_CONTEXT)
>  			audit_log_exit();
>  	}
>  
> @@ -1647,7 +1647,7 @@ void __audit_free(struct task_struct *tsk)
>   * Fill in audit context at syscall entry.  This only happens if the
>   * audit context was created when the task was created and the state or
>   * filters demand the audit context be built.  If the state from the
> - * per-task filter or from the per-syscall filter is AUDIT_RECORD_CONTEXT,
> + * per-task filter or from the per-syscall filter is AUDITSC_RECORD_CONTEXT,
>   * then the record will be written at syscall exit time (otherwise, it
>   * will only be written if another part of the kernel requests that it
>   * be written).
> @@ -1664,11 +1664,11 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
>  	BUG_ON(context->in_syscall || context->name_count);
>  
>  	state = context->state;
> -	if (state == AUDIT_DISABLED)
> +	if (state == AUDITSC_DISABLED)
>  		return;
>  
>  	context->dummy = !audit_n_rules;
> -	if (!context->dummy && state == AUDIT_BUILD_CONTEXT) {
> +	if (!context->dummy && state == AUDITSC_BUILD_CONTEXT) {
>  		context->prio = 0;
>  		if (auditd_test_task(current))
>  			return;
> @@ -1693,7 +1693,7 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
>   * @return_code: return value of the syscall
>   *
>   * Tear down after system call.  If the audit context has been marked as
> - * auditable (either because of the AUDIT_RECORD_CONTEXT state from
> + * auditable (either because of the AUDITSC_RECORD_CONTEXT state from
>   * filtering, or because some other part of the kernel wrote an audit
>   * message), then write out the syscall information.  In call cases,
>   * free the names stored from getname().
> @@ -1735,12 +1735,12 @@ void __audit_syscall_exit(int success, long return_code)
>  
>  		audit_filter_syscall(current, context);
>  		audit_filter_inodes(current, context);
> -		if (context->current_state == AUDIT_RECORD_CONTEXT)
> +		if (context->current_state == AUDITSC_RECORD_CONTEXT)
>  			audit_log_exit();
>  	}
>  
>  	context->in_syscall = 0;
> -	context->prio = context->state == AUDIT_RECORD_CONTEXT ? ~0ULL : 0;
> +	context->prio = context->state == AUDITSC_RECORD_CONTEXT ? ~0ULL : 0;
>  
>  	audit_free_module(context);
>  	audit_free_names(context);
> @@ -1753,7 +1753,7 @@ void __audit_syscall_exit(int success, long return_code)
>  	context->sockaddr_len = 0;
>  	context->type = 0;
>  	context->fds[0] = -1;
> -	if (context->state != AUDIT_RECORD_CONTEXT) {
> +	if (context->state != AUDITSC_RECORD_CONTEXT) {
>  		kfree(context->filterkey);
>  		context->filterkey = NULL;
>  	}
> @@ -2203,7 +2203,7 @@ int auditsc_get_stamp(struct audit_context *ctx,
>  	*serial    = ctx->serial;
>  	if (!ctx->prio) {
>  		ctx->prio = 1;
> -		ctx->current_state = AUDIT_RECORD_CONTEXT;
> +		ctx->current_state = AUDITSC_RECORD_CONTEXT;
>  	}
>  	return 1;
>  }
> -- 
> 1.8.3.1
> 
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://listman.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
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-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] audit: Rename enum audit_state constants to avoid AUDIT_DISABLED redefinition
  2021-06-04 11:21 [PATCH] audit: Rename enum audit_state constants to avoid AUDIT_DISABLED redefinition Sergey Nazarov
  2021-06-04 16:17 ` Richard Guy Briggs
@ 2021-06-06  2:40 ` Paul Moore
  2021-06-07  9:58   ` Sergey Nazarov
  1 sibling, 1 reply; 10+ messages in thread
From: Paul Moore @ 2021-06-06  2:40 UTC (permalink / raw)
  To: Sergey Nazarov; +Cc: linux-audit, linux-kernel, Eric Paris

On Fri, Jun 4, 2021 at 7:21 AM Sergey Nazarov <s-nazarov@yandex.ru> wrote:
>
> AUDIT_DISABLED defined in kernel/audit.h as element of enum audit_state
> and redefined in kernel/audit.c. This produces a warning when kernel builds
> with syscalls audit disabled and brokes kernel build if -Werror used.
> enum audit_state used in syscall audit code only. This patch changes
> enum audit_state constants prefix AUDIT to AUDITSC to avoid AUDIT_DISABLED
> redefinition.
>
> Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
> ---
>  kernel/audit.h   |  8 ++++----
>  kernel/auditsc.c | 34 +++++++++++++++++-----------------
>  2 files changed, 21 insertions(+), 21 deletions(-)

Hi Sergey,

Thanks for sending a patch to fix this problem.  One comment below ...

> diff --git a/kernel/audit.h b/kernel/audit.h
> index 1522e10..ee81f20 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -21,16 +21,16 @@
>     a per-task filter.  At syscall entry, the audit_state is augmented by
>     the syscall filter. */
>  enum audit_state {
> -       AUDIT_DISABLED,         /* Do not create per-task audit_context.
> +       AUDITSC_DISABLED,       /* Do not create per-task audit_context.
>                                  * No syscall-specific audit records can
>                                  * be generated. */
> -       AUDIT_BUILD_CONTEXT,    /* Create the per-task audit_context,
> +       AUDITSC_BUILD_CONTEXT,  /* Create the per-task audit_context,
>                                  * and fill it in at syscall
>                                  * entry time.  This makes a full
>                                  * syscall record available if some
>                                  * other part of the kernel decides it
>                                  * should be recorded. */
> -       AUDIT_RECORD_CONTEXT    /* Create the per-task audit_context,
> +       AUDITSC_RECORD_CONTEXT  /* Create the per-task audit_context,
>                                  * always fill it in at syscall entry
>                                  * time, and always write out the audit
>                                  * record at syscall exit time.  */

I believe that just as the AUDIT_ prefix proved to be a bit too
generic, I think that the AUDITSC_ prefix is also not the best choice.
Would you object to using the AUDIT_STATE_ prefix?  As that may get a
bit long, I might suggest dropping the _CONTEXT from the enums too
such that you would end up with the following:

  enum audit_state {
    AUDIT_STATE_DISABLED,
    AUDIT_STATE_BUILD,
    AUDIT_STATE_RECORD,
  };

Thoughts?

-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] audit: Rename enum audit_state constants to avoid AUDIT_DISABLED redefinition
  2021-06-06  2:40 ` Paul Moore
@ 2021-06-07  9:58   ` Sergey Nazarov
  2021-06-07 17:07     ` Paul Moore
  0 siblings, 1 reply; 10+ messages in thread
From: Sergey Nazarov @ 2021-06-07  9:58 UTC (permalink / raw)
  To: Paul Moore; +Cc: linux-audit, linux-kernel, Eric Paris

Hi, Paul!
I think this could be easer. It's enouth to rename AUDIT_DISABLE only.
enum audit_state deals with per-task syscalls audit context, so we can
use AUDIT_CONTEXT_DISABLED for example. If it's okay, I can send a new
patch version.

В Сб, 05/06/2021 в 22:40 -0400, Paul Moore пишет:
> On Fri, Jun 4, 2021 at 7:21 AM Sergey Nazarov <s-nazarov@yandex.ru>
> wrote:
> > 
> > AUDIT_DISABLED defined in kernel/audit.h as element of enum
> > audit_state
> > and redefined in kernel/audit.c. This produces a warning when
> > kernel builds
> > with syscalls audit disabled and brokes kernel build if -Werror
> > used.
> > enum audit_state used in syscall audit code only. This patch
> > changes
> > enum audit_state constants prefix AUDIT to AUDITSC to avoid
> > AUDIT_DISABLED
> > redefinition.
> > 
> > Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
> > ---
> >  kernel/audit.h   |  8 ++++----
> >  kernel/auditsc.c | 34 +++++++++++++++++-----------------
> >  2 files changed, 21 insertions(+), 21 deletions(-)
> 
> Hi Sergey,
> 
> Thanks for sending a patch to fix this problem.  One comment below
> ...
> 
> > diff --git a/kernel/audit.h b/kernel/audit.h
> > index 1522e10..ee81f20 100644
> > --- a/kernel/audit.h
> > +++ b/kernel/audit.h
> > @@ -21,16 +21,16 @@
> >     a per-task filter.  At syscall entry, the audit_state is
> > augmented by
> >     the syscall filter. */
> >  enum audit_state {
> > -       AUDIT_DISABLED,         /* Do not create per-task
> > audit_context.
> > +       AUDITSC_DISABLED,       /* Do not create per-task
> > audit_context.
> >                                  * No syscall-specific audit
> > records can
> >                                  * be generated. */
> > -       AUDIT_BUILD_CONTEXT,    /* Create the per-task
> > audit_context,
> > +       AUDITSC_BUILD_CONTEXT,  /* Create the per-task
> > audit_context,
> >                                  * and fill it in at syscall
> >                                  * entry time.  This makes a full
> >                                  * syscall record available if some
> >                                  * other part of the kernel decides
> > it
> >                                  * should be recorded. */
> > -       AUDIT_RECORD_CONTEXT    /* Create the per-task
> > audit_context,
> > +       AUDITSC_RECORD_CONTEXT  /* Create the per-task
> > audit_context,
> >                                  * always fill it in at syscall
> > entry
> >                                  * time, and always write out the
> > audit
> >                                  * record at syscall exit time.  */
> 
> I believe that just as the AUDIT_ prefix proved to be a bit too
> generic, I think that the AUDITSC_ prefix is also not the best
> choice.
> Would you object to using the AUDIT_STATE_ prefix?  As that may get a
> bit long, I might suggest dropping the _CONTEXT from the enums too
> such that you would end up with the following:
> 
>   enum audit_state {
>     AUDIT_STATE_DISABLED,
>     AUDIT_STATE_BUILD,
>     AUDIT_STATE_RECORD,
>   };
> 
> Thoughts?
> 


--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] audit: Rename enum audit_state constants to avoid AUDIT_DISABLED redefinition
  2021-06-07  9:58   ` Sergey Nazarov
@ 2021-06-07 17:07     ` Paul Moore
  2021-06-07 17:50       ` Richard Guy Briggs
  0 siblings, 1 reply; 10+ messages in thread
From: Paul Moore @ 2021-06-07 17:07 UTC (permalink / raw)
  To: Sergey Nazarov; +Cc: linux-audit, linux-kernel, Eric Paris

On Mon, Jun 7, 2021 at 5:58 AM Sergey Nazarov <s-nazarov@yandex.ru> wrote:
> Hi, Paul!
> I think this could be easer. It's enouth to rename AUDIT_DISABLE only.
> enum audit_state deals with per-task syscalls audit context, so we can
> use AUDIT_CONTEXT_DISABLED for example. If it's okay, I can send a new
> patch version.

Hi Sergey,

I personally prefer the AUDIT_STATE_* enums and would rather see that.

> В Сб, 05/06/2021 в 22:40 -0400, Paul Moore пишет:
> > On Fri, Jun 4, 2021 at 7:21 AM Sergey Nazarov <s-nazarov@yandex.ru>
> > wrote:
> > >
> > > AUDIT_DISABLED defined in kernel/audit.h as element of enum
> > > audit_state
> > > and redefined in kernel/audit.c. This produces a warning when
> > > kernel builds
> > > with syscalls audit disabled and brokes kernel build if -Werror
> > > used.
> > > enum audit_state used in syscall audit code only. This patch
> > > changes
> > > enum audit_state constants prefix AUDIT to AUDITSC to avoid
> > > AUDIT_DISABLED
> > > redefinition.
> > >
> > > Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
> > > ---
> > >  kernel/audit.h   |  8 ++++----
> > >  kernel/auditsc.c | 34 +++++++++++++++++-----------------
> > >  2 files changed, 21 insertions(+), 21 deletions(-)
> >
> > Hi Sergey,
> >
> > Thanks for sending a patch to fix this problem.  One comment below
> > ...
> >
> > > diff --git a/kernel/audit.h b/kernel/audit.h
> > > index 1522e10..ee81f20 100644
> > > --- a/kernel/audit.h
> > > +++ b/kernel/audit.h
> > > @@ -21,16 +21,16 @@
> > >     a per-task filter.  At syscall entry, the audit_state is
> > > augmented by
> > >     the syscall filter. */
> > >  enum audit_state {
> > > -       AUDIT_DISABLED,         /* Do not create per-task
> > > audit_context.
> > > +       AUDITSC_DISABLED,       /* Do not create per-task
> > > audit_context.
> > >                                  * No syscall-specific audit
> > > records can
> > >                                  * be generated. */
> > > -       AUDIT_BUILD_CONTEXT,    /* Create the per-task
> > > audit_context,
> > > +       AUDITSC_BUILD_CONTEXT,  /* Create the per-task
> > > audit_context,
> > >                                  * and fill it in at syscall
> > >                                  * entry time.  This makes a full
> > >                                  * syscall record available if some
> > >                                  * other part of the kernel decides
> > > it
> > >                                  * should be recorded. */
> > > -       AUDIT_RECORD_CONTEXT    /* Create the per-task
> > > audit_context,
> > > +       AUDITSC_RECORD_CONTEXT  /* Create the per-task
> > > audit_context,
> > >                                  * always fill it in at syscall
> > > entry
> > >                                  * time, and always write out the
> > > audit
> > >                                  * record at syscall exit time.  */
> >
> > I believe that just as the AUDIT_ prefix proved to be a bit too
> > generic, I think that the AUDITSC_ prefix is also not the best
> > choice.
> > Would you object to using the AUDIT_STATE_ prefix?  As that may get a
> > bit long, I might suggest dropping the _CONTEXT from the enums too
> > such that you would end up with the following:
> >
> >   enum audit_state {
> >     AUDIT_STATE_DISABLED,
> >     AUDIT_STATE_BUILD,
> >     AUDIT_STATE_RECORD,
> >   };
> >
> > Thoughts?
> >
>


-- 
paul moore
www.paul-moore.com


--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] audit: Rename enum audit_state constants to avoid AUDIT_DISABLED redefinition
  2021-06-07 17:07     ` Paul Moore
@ 2021-06-07 17:50       ` Richard Guy Briggs
  2021-06-07 18:18         ` Paul Moore
  0 siblings, 1 reply; 10+ messages in thread
From: Richard Guy Briggs @ 2021-06-07 17:50 UTC (permalink / raw)
  To: Paul Moore; +Cc: Sergey Nazarov, linux-audit, linux-kernel, Eric Paris

On 2021-06-07 13:07, Paul Moore wrote:
> On Mon, Jun 7, 2021 at 5:58 AM Sergey Nazarov <s-nazarov@yandex.ru> wrote:
> > Hi, Paul!
> > I think this could be easer. It's enouth to rename AUDIT_DISABLE only.
> > enum audit_state deals with per-task syscalls audit context, so we can
> > use AUDIT_CONTEXT_DISABLED for example. If it's okay, I can send a new
> > patch version.
> 
> Hi Sergey,
> 
> I personally prefer the AUDIT_STATE_* enums and would rather see that.

I find AUDITSC_* or AUDIT*CONTEXT* are more helpful since that makes it
clear it is about syscall context while STATE doesn't make that clear.

The minimal change could have been to AUDIT_DISABLE_CONTEXT.

> > В Сб, 05/06/2021 в 22:40 -0400, Paul Moore пишет:
> > > On Fri, Jun 4, 2021 at 7:21 AM Sergey Nazarov <s-nazarov@yandex.ru>
> > > wrote:
> > > >
> > > > AUDIT_DISABLED defined in kernel/audit.h as element of enum
> > > > audit_state
> > > > and redefined in kernel/audit.c. This produces a warning when
> > > > kernel builds
> > > > with syscalls audit disabled and brokes kernel build if -Werror
> > > > used.
> > > > enum audit_state used in syscall audit code only. This patch
> > > > changes
> > > > enum audit_state constants prefix AUDIT to AUDITSC to avoid
> > > > AUDIT_DISABLED
> > > > redefinition.
> > > >
> > > > Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
> > > > ---
> > > >  kernel/audit.h   |  8 ++++----
> > > >  kernel/auditsc.c | 34 +++++++++++++++++-----------------
> > > >  2 files changed, 21 insertions(+), 21 deletions(-)
> > >
> > > Hi Sergey,
> > >
> > > Thanks for sending a patch to fix this problem.  One comment below
> > > ...
> > >
> > > > diff --git a/kernel/audit.h b/kernel/audit.h
> > > > index 1522e10..ee81f20 100644
> > > > --- a/kernel/audit.h
> > > > +++ b/kernel/audit.h
> > > > @@ -21,16 +21,16 @@
> > > >     a per-task filter.  At syscall entry, the audit_state is
> > > > augmented by
> > > >     the syscall filter. */
> > > >  enum audit_state {
> > > > -       AUDIT_DISABLED,         /* Do not create per-task
> > > > audit_context.
> > > > +       AUDITSC_DISABLED,       /* Do not create per-task
> > > > audit_context.
> > > >                                  * No syscall-specific audit
> > > > records can
> > > >                                  * be generated. */
> > > > -       AUDIT_BUILD_CONTEXT,    /* Create the per-task
> > > > audit_context,
> > > > +       AUDITSC_BUILD_CONTEXT,  /* Create the per-task
> > > > audit_context,
> > > >                                  * and fill it in at syscall
> > > >                                  * entry time.  This makes a full
> > > >                                  * syscall record available if some
> > > >                                  * other part of the kernel decides
> > > > it
> > > >                                  * should be recorded. */
> > > > -       AUDIT_RECORD_CONTEXT    /* Create the per-task
> > > > audit_context,
> > > > +       AUDITSC_RECORD_CONTEXT  /* Create the per-task
> > > > audit_context,
> > > >                                  * always fill it in at syscall
> > > > entry
> > > >                                  * time, and always write out the
> > > > audit
> > > >                                  * record at syscall exit time.  */
> > >
> > > I believe that just as the AUDIT_ prefix proved to be a bit too
> > > generic, I think that the AUDITSC_ prefix is also not the best
> > > choice.
> > > Would you object to using the AUDIT_STATE_ prefix?  As that may get a
> > > bit long, I might suggest dropping the _CONTEXT from the enums too
> > > such that you would end up with the following:
> > >
> > >   enum audit_state {
> > >     AUDIT_STATE_DISABLED,
> > >     AUDIT_STATE_BUILD,
> > >     AUDIT_STATE_RECORD,
> > >   };
> > >
> > > Thoughts?
> 
> paul moore

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
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-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] audit: Rename enum audit_state constants to avoid AUDIT_DISABLED redefinition
  2021-06-07 17:50       ` Richard Guy Briggs
@ 2021-06-07 18:18         ` Paul Moore
  2021-06-08  6:32           ` [PATCH v2] " Sergey Nazarov
  0 siblings, 1 reply; 10+ messages in thread
From: Paul Moore @ 2021-06-07 18:18 UTC (permalink / raw)
  To: Richard Guy Briggs; +Cc: Sergey Nazarov, linux-audit, linux-kernel, Eric Paris

On Mon, Jun 7, 2021 at 1:51 PM Richard Guy Briggs <rgb@redhat.com> wrote:
> On 2021-06-07 13:07, Paul Moore wrote:
> > On Mon, Jun 7, 2021 at 5:58 AM Sergey Nazarov <s-nazarov@yandex.ru> wrote:
> > > Hi, Paul!
> > > I think this could be easer. It's enouth to rename AUDIT_DISABLE only.
> > > enum audit_state deals with per-task syscalls audit context, so we can
> > > use AUDIT_CONTEXT_DISABLED for example. If it's okay, I can send a new
> > > patch version.
> >
> > Hi Sergey,
> >
> > I personally prefer the AUDIT_STATE_* enums and would rather see that.
>
> I find AUDITSC_* or AUDIT*CONTEXT* are more helpful since that makes it
> clear it is about syscall context while STATE doesn't make that clear.

We've seen how this will grow to incorporate more than just a syscall
context, see the io_uring work that is ongoing.

Let me be very clear this time, please use the AUDIT_STATE_* enums.

-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit


^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH v2] audit: Rename enum audit_state constants to avoid AUDIT_DISABLED redefinition
  2021-06-07 18:18         ` Paul Moore
@ 2021-06-08  6:32           ` Sergey Nazarov
  2021-06-08 15:04             ` Richard Guy Briggs
  2021-06-09  2:12             ` Paul Moore
  0 siblings, 2 replies; 10+ messages in thread
From: Sergey Nazarov @ 2021-06-08  6:32 UTC (permalink / raw)
  To: Paul Moore, Richard Guy Briggs; +Cc: linux-audit, linux-kernel, Eric Paris

AUDIT_DISABLED defined in kernel/audit.h as element of enum audit_state
and redefined in kernel/audit.c. This produces a warning when kernel builds
with syscalls audit disabled and brokes kernel build if -Werror used.
enum audit_state used in syscall audit code only. This patch changes
enum audit_state constants prefix AUDIT to AUDIT_STATE to avoid
AUDIT_DISABLED redefinition.

v2: the comments of Richard Guy Briggs and Paul Moore were taken into account

Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
---
 kernel/audit.h   |  8 ++++----
 kernel/auditsc.c | 34 +++++++++++++++++-----------------
 2 files changed, 21 insertions(+), 21 deletions(-)

diff --git a/kernel/audit.h b/kernel/audit.h
index 1522e10..e518ad9 100644
--- a/kernel/audit.h
+++ b/kernel/audit.h
@@ -21,16 +21,16 @@
    a per-task filter.  At syscall entry, the audit_state is augmented by
    the syscall filter. */
 enum audit_state {
-	AUDIT_DISABLED,		/* Do not create per-task audit_context.
+	AUDIT_STATE_DISABLED,	/* Do not create per-task audit_context.
 				 * No syscall-specific audit records can
 				 * be generated. */
-	AUDIT_BUILD_CONTEXT,	/* Create the per-task audit_context,
+	AUDIT_STATE_BUILD,	/* Create the per-task audit_context,
 				 * and fill it in at syscall
 				 * entry time.  This makes a full
 				 * syscall record available if some
 				 * other part of the kernel decides it
 				 * should be recorded. */
-	AUDIT_RECORD_CONTEXT	/* Create the per-task audit_context,
+	AUDIT_STATE_RECORD	/* Create the per-task audit_context,
 				 * always fill it in at syscall entry
 				 * time, and always write out the audit
 				 * record at syscall exit time.  */
@@ -322,7 +322,7 @@ static inline int audit_signal_info_syscall(struct task_struct *t)
 	return 0;
 }
 
-#define audit_filter_inodes(t, c) AUDIT_DISABLED
+#define audit_filter_inodes(t, c) AUDIT_STATE_DISABLED
 #endif /* CONFIG_AUDITSYSCALL */
 
 extern char *audit_unpack_string(void **bufp, size_t *remain, size_t len);
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 175ef6f..92ca5a2 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -231,7 +231,7 @@ static void audit_set_auditable(struct audit_context *ctx)
 {
 	if (!ctx->prio) {
 		ctx->prio = 1;
-		ctx->current_state = AUDIT_RECORD_CONTEXT;
+		ctx->current_state = AUDIT_STATE_RECORD;
 	}
 }
 
@@ -751,10 +751,10 @@ static int audit_filter_rules(struct task_struct *tsk,
 	}
 	switch (rule->action) {
 	case AUDIT_NEVER:
-		*state = AUDIT_DISABLED;
+		*state = AUDIT_STATE_DISABLED;
 		break;
 	case AUDIT_ALWAYS:
-		*state = AUDIT_RECORD_CONTEXT;
+		*state = AUDIT_STATE_RECORD;
 		break;
 	}
 	return 1;
@@ -773,14 +773,14 @@ static enum audit_state audit_filter_task(struct task_struct *tsk, char **key)
 	list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TASK], list) {
 		if (audit_filter_rules(tsk, &e->rule, NULL, NULL,
 				       &state, true)) {
-			if (state == AUDIT_RECORD_CONTEXT)
+			if (state == AUDIT_STATE_RECORD)
 				*key = kstrdup(e->rule.filterkey, GFP_ATOMIC);
 			rcu_read_unlock();
 			return state;
 		}
 	}
 	rcu_read_unlock();
-	return AUDIT_BUILD_CONTEXT;
+	return AUDIT_STATE_BUILD;
 }
 
 static int audit_in_mask(const struct audit_krule *rule, unsigned long val)
@@ -802,7 +802,7 @@ static int audit_in_mask(const struct audit_krule *rule, unsigned long val)
 /* At syscall exit time, this filter is called if the audit_state is
  * not low enough that auditing cannot take place, but is also not
  * high enough that we already know we have to write an audit record
- * (i.e., the state is AUDIT_SETUP_CONTEXT or AUDIT_BUILD_CONTEXT).
+ * (i.e., the state is AUDIT_STATE_BUILD).
  */
 static void audit_filter_syscall(struct task_struct *tsk,
 				 struct audit_context *ctx)
@@ -923,7 +923,7 @@ static inline struct audit_context *audit_alloc_context(enum audit_state state)
 	if (!context)
 		return NULL;
 	context->state = state;
-	context->prio = state == AUDIT_RECORD_CONTEXT ? ~0ULL : 0;
+	context->prio = state == AUDIT_STATE_RECORD ? ~0ULL : 0;
 	INIT_LIST_HEAD(&context->killed_trees);
 	INIT_LIST_HEAD(&context->names_list);
 	context->fds[0] = -1;
@@ -950,7 +950,7 @@ int audit_alloc(struct task_struct *tsk)
 		return 0; /* Return if not auditing. */
 
 	state = audit_filter_task(tsk, &key);
-	if (state == AUDIT_DISABLED) {
+	if (state == AUDIT_STATE_DISABLED) {
 		clear_task_syscall_work(tsk, SYSCALL_AUDIT);
 		return 0;
 	}
@@ -1628,7 +1628,7 @@ void __audit_free(struct task_struct *tsk)
 
 		audit_filter_syscall(tsk, context);
 		audit_filter_inodes(tsk, context);
-		if (context->current_state == AUDIT_RECORD_CONTEXT)
+		if (context->current_state == AUDIT_STATE_RECORD)
 			audit_log_exit();
 	}
 
@@ -1647,7 +1647,7 @@ void __audit_free(struct task_struct *tsk)
  * Fill in audit context at syscall entry.  This only happens if the
  * audit context was created when the task was created and the state or
  * filters demand the audit context be built.  If the state from the
- * per-task filter or from the per-syscall filter is AUDIT_RECORD_CONTEXT,
+ * per-task filter or from the per-syscall filter is AUDIT_STATE_RECORD,
  * then the record will be written at syscall exit time (otherwise, it
  * will only be written if another part of the kernel requests that it
  * be written).
@@ -1664,11 +1664,11 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
 	BUG_ON(context->in_syscall || context->name_count);
 
 	state = context->state;
-	if (state == AUDIT_DISABLED)
+	if (state == AUDIT_STATE_DISABLED)
 		return;
 
 	context->dummy = !audit_n_rules;
-	if (!context->dummy && state == AUDIT_BUILD_CONTEXT) {
+	if (!context->dummy && state == AUDIT_STATE_BUILD) {
 		context->prio = 0;
 		if (auditd_test_task(current))
 			return;
@@ -1693,7 +1693,7 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
  * @return_code: return value of the syscall
  *
  * Tear down after system call.  If the audit context has been marked as
- * auditable (either because of the AUDIT_RECORD_CONTEXT state from
+ * auditable (either because of the AUDIT_STATE_RECORD state from
  * filtering, or because some other part of the kernel wrote an audit
  * message), then write out the syscall information.  In call cases,
  * free the names stored from getname().
@@ -1735,12 +1735,12 @@ void __audit_syscall_exit(int success, long return_code)
 
 		audit_filter_syscall(current, context);
 		audit_filter_inodes(current, context);
-		if (context->current_state == AUDIT_RECORD_CONTEXT)
+		if (context->current_state == AUDIT_STATE_RECORD)
 			audit_log_exit();
 	}
 
 	context->in_syscall = 0;
-	context->prio = context->state == AUDIT_RECORD_CONTEXT ? ~0ULL : 0;
+	context->prio = context->state == AUDIT_STATE_RECORD ? ~0ULL : 0;
 
 	audit_free_module(context);
 	audit_free_names(context);
@@ -1753,7 +1753,7 @@ void __audit_syscall_exit(int success, long return_code)
 	context->sockaddr_len = 0;
 	context->type = 0;
 	context->fds[0] = -1;
-	if (context->state != AUDIT_RECORD_CONTEXT) {
+	if (context->state != AUDIT_STATE_RECORD) {
 		kfree(context->filterkey);
 		context->filterkey = NULL;
 	}
@@ -2203,7 +2203,7 @@ int auditsc_get_stamp(struct audit_context *ctx,
 	*serial    = ctx->serial;
 	if (!ctx->prio) {
 		ctx->prio = 1;
-		ctx->current_state = AUDIT_RECORD_CONTEXT;
+		ctx->current_state = AUDIT_STATE_RECORD;
 	}
 	return 1;
 }
-- 
1.8.3.1


--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] audit: Rename enum audit_state constants to avoid AUDIT_DISABLED redefinition
  2021-06-08  6:32           ` [PATCH v2] " Sergey Nazarov
@ 2021-06-08 15:04             ` Richard Guy Briggs
  2021-06-09  2:12             ` Paul Moore
  1 sibling, 0 replies; 10+ messages in thread
From: Richard Guy Briggs @ 2021-06-08 15:04 UTC (permalink / raw)
  To: Sergey Nazarov; +Cc: linux-audit, Eric Paris, linux-kernel

On 2021-06-08 09:32, Sergey Nazarov wrote:
> AUDIT_DISABLED defined in kernel/audit.h as element of enum audit_state
> and redefined in kernel/audit.c. This produces a warning when kernel builds
> with syscalls audit disabled and brokes kernel build if -Werror used.
> enum audit_state used in syscall audit code only. This patch changes
> enum audit_state constants prefix AUDIT to AUDIT_STATE to avoid
> AUDIT_DISABLED redefinition.
> 
> v2: the comments of Richard Guy Briggs and Paul Moore were taken into account
> 
> Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>

Acked-by: Richard Guy Briggs <rgb@redhat.com>

> ---
>  kernel/audit.h   |  8 ++++----
>  kernel/auditsc.c | 34 +++++++++++++++++-----------------
>  2 files changed, 21 insertions(+), 21 deletions(-)
> 
> diff --git a/kernel/audit.h b/kernel/audit.h
> index 1522e10..e518ad9 100644
> --- a/kernel/audit.h
> +++ b/kernel/audit.h
> @@ -21,16 +21,16 @@
>     a per-task filter.  At syscall entry, the audit_state is augmented by
>     the syscall filter. */
>  enum audit_state {
> -	AUDIT_DISABLED,		/* Do not create per-task audit_context.
> +	AUDIT_STATE_DISABLED,	/* Do not create per-task audit_context.
>  				 * No syscall-specific audit records can
>  				 * be generated. */
> -	AUDIT_BUILD_CONTEXT,	/* Create the per-task audit_context,
> +	AUDIT_STATE_BUILD,	/* Create the per-task audit_context,
>  				 * and fill it in at syscall
>  				 * entry time.  This makes a full
>  				 * syscall record available if some
>  				 * other part of the kernel decides it
>  				 * should be recorded. */
> -	AUDIT_RECORD_CONTEXT	/* Create the per-task audit_context,
> +	AUDIT_STATE_RECORD	/* Create the per-task audit_context,
>  				 * always fill it in at syscall entry
>  				 * time, and always write out the audit
>  				 * record at syscall exit time.  */
> @@ -322,7 +322,7 @@ static inline int audit_signal_info_syscall(struct task_struct *t)
>  	return 0;
>  }
>  
> -#define audit_filter_inodes(t, c) AUDIT_DISABLED
> +#define audit_filter_inodes(t, c) AUDIT_STATE_DISABLED
>  #endif /* CONFIG_AUDITSYSCALL */
>  
>  extern char *audit_unpack_string(void **bufp, size_t *remain, size_t len);
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 175ef6f..92ca5a2 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -231,7 +231,7 @@ static void audit_set_auditable(struct audit_context *ctx)
>  {
>  	if (!ctx->prio) {
>  		ctx->prio = 1;
> -		ctx->current_state = AUDIT_RECORD_CONTEXT;
> +		ctx->current_state = AUDIT_STATE_RECORD;
>  	}
>  }
>  
> @@ -751,10 +751,10 @@ static int audit_filter_rules(struct task_struct *tsk,
>  	}
>  	switch (rule->action) {
>  	case AUDIT_NEVER:
> -		*state = AUDIT_DISABLED;
> +		*state = AUDIT_STATE_DISABLED;
>  		break;
>  	case AUDIT_ALWAYS:
> -		*state = AUDIT_RECORD_CONTEXT;
> +		*state = AUDIT_STATE_RECORD;
>  		break;
>  	}
>  	return 1;
> @@ -773,14 +773,14 @@ static enum audit_state audit_filter_task(struct task_struct *tsk, char **key)
>  	list_for_each_entry_rcu(e, &audit_filter_list[AUDIT_FILTER_TASK], list) {
>  		if (audit_filter_rules(tsk, &e->rule, NULL, NULL,
>  				       &state, true)) {
> -			if (state == AUDIT_RECORD_CONTEXT)
> +			if (state == AUDIT_STATE_RECORD)
>  				*key = kstrdup(e->rule.filterkey, GFP_ATOMIC);
>  			rcu_read_unlock();
>  			return state;
>  		}
>  	}
>  	rcu_read_unlock();
> -	return AUDIT_BUILD_CONTEXT;
> +	return AUDIT_STATE_BUILD;
>  }
>  
>  static int audit_in_mask(const struct audit_krule *rule, unsigned long val)
> @@ -802,7 +802,7 @@ static int audit_in_mask(const struct audit_krule *rule, unsigned long val)
>  /* At syscall exit time, this filter is called if the audit_state is
>   * not low enough that auditing cannot take place, but is also not
>   * high enough that we already know we have to write an audit record
> - * (i.e., the state is AUDIT_SETUP_CONTEXT or AUDIT_BUILD_CONTEXT).
> + * (i.e., the state is AUDIT_STATE_BUILD).
>   */
>  static void audit_filter_syscall(struct task_struct *tsk,
>  				 struct audit_context *ctx)
> @@ -923,7 +923,7 @@ static inline struct audit_context *audit_alloc_context(enum audit_state state)
>  	if (!context)
>  		return NULL;
>  	context->state = state;
> -	context->prio = state == AUDIT_RECORD_CONTEXT ? ~0ULL : 0;
> +	context->prio = state == AUDIT_STATE_RECORD ? ~0ULL : 0;
>  	INIT_LIST_HEAD(&context->killed_trees);
>  	INIT_LIST_HEAD(&context->names_list);
>  	context->fds[0] = -1;
> @@ -950,7 +950,7 @@ int audit_alloc(struct task_struct *tsk)
>  		return 0; /* Return if not auditing. */
>  
>  	state = audit_filter_task(tsk, &key);
> -	if (state == AUDIT_DISABLED) {
> +	if (state == AUDIT_STATE_DISABLED) {
>  		clear_task_syscall_work(tsk, SYSCALL_AUDIT);
>  		return 0;
>  	}
> @@ -1628,7 +1628,7 @@ void __audit_free(struct task_struct *tsk)
>  
>  		audit_filter_syscall(tsk, context);
>  		audit_filter_inodes(tsk, context);
> -		if (context->current_state == AUDIT_RECORD_CONTEXT)
> +		if (context->current_state == AUDIT_STATE_RECORD)
>  			audit_log_exit();
>  	}
>  
> @@ -1647,7 +1647,7 @@ void __audit_free(struct task_struct *tsk)
>   * Fill in audit context at syscall entry.  This only happens if the
>   * audit context was created when the task was created and the state or
>   * filters demand the audit context be built.  If the state from the
> - * per-task filter or from the per-syscall filter is AUDIT_RECORD_CONTEXT,
> + * per-task filter or from the per-syscall filter is AUDIT_STATE_RECORD,
>   * then the record will be written at syscall exit time (otherwise, it
>   * will only be written if another part of the kernel requests that it
>   * be written).
> @@ -1664,11 +1664,11 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
>  	BUG_ON(context->in_syscall || context->name_count);
>  
>  	state = context->state;
> -	if (state == AUDIT_DISABLED)
> +	if (state == AUDIT_STATE_DISABLED)
>  		return;
>  
>  	context->dummy = !audit_n_rules;
> -	if (!context->dummy && state == AUDIT_BUILD_CONTEXT) {
> +	if (!context->dummy && state == AUDIT_STATE_BUILD) {
>  		context->prio = 0;
>  		if (auditd_test_task(current))
>  			return;
> @@ -1693,7 +1693,7 @@ void __audit_syscall_entry(int major, unsigned long a1, unsigned long a2,
>   * @return_code: return value of the syscall
>   *
>   * Tear down after system call.  If the audit context has been marked as
> - * auditable (either because of the AUDIT_RECORD_CONTEXT state from
> + * auditable (either because of the AUDIT_STATE_RECORD state from
>   * filtering, or because some other part of the kernel wrote an audit
>   * message), then write out the syscall information.  In call cases,
>   * free the names stored from getname().
> @@ -1735,12 +1735,12 @@ void __audit_syscall_exit(int success, long return_code)
>  
>  		audit_filter_syscall(current, context);
>  		audit_filter_inodes(current, context);
> -		if (context->current_state == AUDIT_RECORD_CONTEXT)
> +		if (context->current_state == AUDIT_STATE_RECORD)
>  			audit_log_exit();
>  	}
>  
>  	context->in_syscall = 0;
> -	context->prio = context->state == AUDIT_RECORD_CONTEXT ? ~0ULL : 0;
> +	context->prio = context->state == AUDIT_STATE_RECORD ? ~0ULL : 0;
>  
>  	audit_free_module(context);
>  	audit_free_names(context);
> @@ -1753,7 +1753,7 @@ void __audit_syscall_exit(int success, long return_code)
>  	context->sockaddr_len = 0;
>  	context->type = 0;
>  	context->fds[0] = -1;
> -	if (context->state != AUDIT_RECORD_CONTEXT) {
> +	if (context->state != AUDIT_STATE_RECORD) {
>  		kfree(context->filterkey);
>  		context->filterkey = NULL;
>  	}
> @@ -2203,7 +2203,7 @@ int auditsc_get_stamp(struct audit_context *ctx,
>  	*serial    = ctx->serial;
>  	if (!ctx->prio) {
>  		ctx->prio = 1;
> -		ctx->current_state = AUDIT_RECORD_CONTEXT;
> +		ctx->current_state = AUDIT_STATE_RECORD;
>  	}
>  	return 1;
>  }
> -- 
> 1.8.3.1
> 
> 
> --
> Linux-audit mailing list
> Linux-audit@redhat.com
> https://listman.redhat.com/mailman/listinfo/linux-audit

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
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-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit


^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH v2] audit: Rename enum audit_state constants to avoid AUDIT_DISABLED redefinition
  2021-06-08  6:32           ` [PATCH v2] " Sergey Nazarov
  2021-06-08 15:04             ` Richard Guy Briggs
@ 2021-06-09  2:12             ` Paul Moore
  1 sibling, 0 replies; 10+ messages in thread
From: Paul Moore @ 2021-06-09  2:12 UTC (permalink / raw)
  To: Sergey Nazarov; +Cc: Richard Guy Briggs, linux-audit, linux-kernel, Eric Paris

On Tue, Jun 8, 2021 at 2:32 AM Sergey Nazarov <s-nazarov@yandex.ru> wrote:
>
> AUDIT_DISABLED defined in kernel/audit.h as element of enum audit_state
> and redefined in kernel/audit.c. This produces a warning when kernel builds
> with syscalls audit disabled and brokes kernel build if -Werror used.
> enum audit_state used in syscall audit code only. This patch changes
> enum audit_state constants prefix AUDIT to AUDIT_STATE to avoid
> AUDIT_DISABLED redefinition.
>
> v2: the comments of Richard Guy Briggs and Paul Moore were taken into account
>
> Signed-off-by: Sergey Nazarov <s-nazarov@yandex.ru>
> ---
>  kernel/audit.h   |  8 ++++----
>  kernel/auditsc.c | 34 +++++++++++++++++-----------------
>  2 files changed, 21 insertions(+), 21 deletions(-)

Merged into audit/next, thanks Sergey.

-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://listman.redhat.com/mailman/listinfo/linux-audit


^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2021-06-09  2:13 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-04 11:21 [PATCH] audit: Rename enum audit_state constants to avoid AUDIT_DISABLED redefinition Sergey Nazarov
2021-06-04 16:17 ` Richard Guy Briggs
2021-06-06  2:40 ` Paul Moore
2021-06-07  9:58   ` Sergey Nazarov
2021-06-07 17:07     ` Paul Moore
2021-06-07 17:50       ` Richard Guy Briggs
2021-06-07 18:18         ` Paul Moore
2021-06-08  6:32           ` [PATCH v2] " Sergey Nazarov
2021-06-08 15:04             ` Richard Guy Briggs
2021-06-09  2:12             ` Paul Moore

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).