[RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL

[RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL

[Richard Guy Briggs]

The AUDIT_FILTER_TYPE name is vague and misleading due to not describing
where or when the filter is applied and obsolete due to its available
filter fields having been expanded.

Userspace has already renamed it from AUDIT_FILTER_TYPE to
AUDIT_FILTER_EXCLUDE without checking if it already exists. In order to
not cause userspace compile problems from duplicate definitions and to
more accurately and inclusively rename it in the kernel, while providing
a migration path for userspace, rename it to AUDIT_FILTER_EXCL.

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

Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
---
 include/uapi/linux/audit.h |  3 ++-
 kernel/audit.c             |  2 +-
 kernel/auditfilter.c       | 10 +++++-----
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
index 04f9bd2..45dd7ef 100644
--- a/include/uapi/linux/audit.h
+++ b/include/uapi/linux/audit.h
@@ -156,8 +156,9 @@
 #define AUDIT_FILTER_ENTRY	0x02	/* Apply rule at syscall entry */
 #define AUDIT_FILTER_WATCH	0x03	/* Apply rule to file system watches */
 #define AUDIT_FILTER_EXIT	0x04	/* Apply rule at syscall exit */
-#define AUDIT_FILTER_TYPE	0x05	/* Apply rule at audit_log_start */
+#define AUDIT_FILTER_EXCL	0x05	/* Apply rule at audit_log_start */
 #define AUDIT_FILTER_FS		0x06	/* Apply rule at __audit_inode_child */
+#define AUDIT_FILTER_TYPE	AUDIT_FILTER_EXCL /* obsolete misleading naming */
 
 #define AUDIT_NR_FILTERS	7
 
diff --git a/kernel/audit.c b/kernel/audit.c
index 3a18e59..089cede 100644
--- a/kernel/audit.c
+++ b/kernel/audit.c
@@ -1754,7 +1754,7 @@ struct audit_buffer *audit_log_start(struct audit_context *ctx, gfp_t gfp_mask,
 	if (audit_initialized != AUDIT_INITIALIZED)
 		return NULL;
 
-	if (unlikely(!audit_filter(type, AUDIT_FILTER_TYPE)))
+	if (unlikely(!audit_filter(type, AUDIT_FILTER_EXCL)))
 		return NULL;
 
 	/* NOTE: don't ever fail/sleep on these two conditions:
diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
index eaa3201..f17a42f5 100644
--- a/kernel/auditfilter.c
+++ b/kernel/auditfilter.c
@@ -264,7 +264,7 @@ static inline struct audit_entry *audit_to_entry_common(struct audit_rule_data *
 	case AUDIT_FILTER_TASK:
 #endif
 	case AUDIT_FILTER_USER:
-	case AUDIT_FILTER_TYPE:
+	case AUDIT_FILTER_EXCL:
 	case AUDIT_FILTER_FS:
 		;
 	}
@@ -337,7 +337,7 @@ static int audit_field_valid(struct audit_entry *entry, struct audit_field *f)
 {
 	switch(f->type) {
 	case AUDIT_MSGTYPE:
-		if (entry->rule.listnr != AUDIT_FILTER_TYPE &&
+		if (entry->rule.listnr != AUDIT_FILTER_EXCL &&
 		    entry->rule.listnr != AUDIT_FILTER_USER)
 			return -EINVAL;
 		break;
@@ -931,7 +931,7 @@ static inline int audit_add_rule(struct audit_entry *entry)
 	/* If any of these, don't count towards total */
 	switch(entry->rule.listnr) {
 	case AUDIT_FILTER_USER:
-	case AUDIT_FILTER_TYPE:
+	case AUDIT_FILTER_EXCL:
 	case AUDIT_FILTER_FS:
 		dont_count = 1;
 	}
@@ -1013,7 +1013,7 @@ int audit_del_rule(struct audit_entry *entry)
 	/* If any of these, don't count towards total */
 	switch(entry->rule.listnr) {
 	case AUDIT_FILTER_USER:
-	case AUDIT_FILTER_TYPE:
+	case AUDIT_FILTER_EXCL:
 	case AUDIT_FILTER_FS:
 		dont_count = 1;
 	}
@@ -1369,7 +1369,7 @@ int audit_filter(int msgtype, unsigned int listtype)
 				break;
 		}
 		if (result > 0) {
-			if (e->rule.action == AUDIT_NEVER || listtype == AUDIT_FILTER_TYPE)
+			if (e->rule.action == AUDIT_NEVER || listtype == AUDIT_FILTER_EXCL)
 				ret = 0;
 			break;
 		}
-- 
1.8.3.1

Re: [RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL

[Steve Grubb]

On Thursday, May 31, 2018 4:23:09 PM EDT Richard Guy Briggs wrote:
> The AUDIT_FILTER_TYPE name is vague and misleading due to not describing
> where or when the filter is applied and obsolete due to its available
> filter fields having been expanded.
> 
> Userspace has already renamed it from AUDIT_FILTER_TYPE to
> AUDIT_FILTER_EXCLUDE without checking if it already exists.

Historically speaking, this is not why it is the way it is. But I think it 
doesn't mean that you cannot do something like this:

#define AUDIT_FILTER_EXCLUDE    AUDIT_FILTER_TYPE

It's easy then to add a #ifndef to the userspace code so that there is an 
easy migration. I also do not see any compiler warnings with the above in 
both /usr/include/linux/audit.h and /usr/include/libaudit.h.

-Steve

> In order to
> not cause userspace compile problems from duplicate definitions and to
> more accurately and inclusively rename it in the kernel, while providing
> a migration path for userspace, rename it to AUDIT_FILTER_EXCL.
> 
> See: https://github.com/linux-audit/audit-kernel/issues/89
> 
> Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> ---
>  include/uapi/linux/audit.h |  3 ++-
>  kernel/audit.c             |  2 +-
>  kernel/auditfilter.c       | 10 +++++-----
>  3 files changed, 8 insertions(+), 7 deletions(-)
> 
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 04f9bd2..45dd7ef 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -156,8 +156,9 @@
>  #define AUDIT_FILTER_ENTRY	0x02	/* Apply rule at syscall entry */
>  #define AUDIT_FILTER_WATCH	0x03	/* Apply rule to file system watches */
>  #define AUDIT_FILTER_EXIT	0x04	/* Apply rule at syscall exit */
> -#define AUDIT_FILTER_TYPE	0x05	/* Apply rule at audit_log_start */
> +#define AUDIT_FILTER_EXCL	0x05	/* Apply rule at audit_log_start */
>  #define AUDIT_FILTER_FS		0x06	/* Apply rule at __audit_inode_child */
> +#define AUDIT_FILTER_TYPE	AUDIT_FILTER_EXCL /* obsolete misleading naming
> */
> 
>  #define AUDIT_NR_FILTERS	7
> 
> diff --git a/kernel/audit.c b/kernel/audit.c
> index 3a18e59..089cede 100644
> --- a/kernel/audit.c
> +++ b/kernel/audit.c
> @@ -1754,7 +1754,7 @@ struct audit_buffer *audit_log_start(struct
> audit_context *ctx, gfp_t gfp_mask, if (audit_initialized !=
> AUDIT_INITIALIZED)
>  		return NULL;
> 
> -	if (unlikely(!audit_filter(type, AUDIT_FILTER_TYPE)))
> +	if (unlikely(!audit_filter(type, AUDIT_FILTER_EXCL)))
>  		return NULL;
> 
>  	/* NOTE: don't ever fail/sleep on these two conditions:
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index eaa3201..f17a42f5 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -264,7 +264,7 @@ static inline struct audit_entry
> *audit_to_entry_common(struct audit_rule_data * case AUDIT_FILTER_TASK:
>  #endif
>  	case AUDIT_FILTER_USER:
> -	case AUDIT_FILTER_TYPE:
> +	case AUDIT_FILTER_EXCL:
>  	case AUDIT_FILTER_FS:
>  		;
>  	}
> @@ -337,7 +337,7 @@ static int audit_field_valid(struct audit_entry *entry,
> struct audit_field *f) {
>  	switch(f->type) {
>  	case AUDIT_MSGTYPE:
> -		if (entry->rule.listnr != AUDIT_FILTER_TYPE &&
> +		if (entry->rule.listnr != AUDIT_FILTER_EXCL &&
>  		    entry->rule.listnr != AUDIT_FILTER_USER)
>  			return -EINVAL;
>  		break;
> @@ -931,7 +931,7 @@ static inline int audit_add_rule(struct audit_entry
> *entry) /* If any of these, don't count towards total */
>  	switch(entry->rule.listnr) {
>  	case AUDIT_FILTER_USER:
> -	case AUDIT_FILTER_TYPE:
> +	case AUDIT_FILTER_EXCL:
>  	case AUDIT_FILTER_FS:
>  		dont_count = 1;
>  	}
> @@ -1013,7 +1013,7 @@ int audit_del_rule(struct audit_entry *entry)
>  	/* If any of these, don't count towards total */
>  	switch(entry->rule.listnr) {
>  	case AUDIT_FILTER_USER:
> -	case AUDIT_FILTER_TYPE:
> +	case AUDIT_FILTER_EXCL:
>  	case AUDIT_FILTER_FS:
>  		dont_count = 1;
>  	}
> @@ -1369,7 +1369,7 @@ int audit_filter(int msgtype, unsigned int listtype)
>  				break;
>  		}
>  		if (result > 0) {
> -			if (e->rule.action == AUDIT_NEVER || listtype == AUDIT_FILTER_TYPE)
> +			if (e->rule.action == AUDIT_NEVER || listtype == AUDIT_FILTER_EXCL)
>  				ret = 0;
>  			break;
>  		}

Re: [RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL

[Richard Guy Briggs]

On 2018-05-31 17:29, Steve Grubb wrote:
> On Thursday, May 31, 2018 4:23:09 PM EDT Richard Guy Briggs wrote:
> > The AUDIT_FILTER_TYPE name is vague and misleading due to not describing
> > where or when the filter is applied and obsolete due to its available
> > filter fields having been expanded.
> > 
> > Userspace has already renamed it from AUDIT_FILTER_TYPE to
> > AUDIT_FILTER_EXCLUDE without checking if it already exists.
> 
> Historically speaking, this is not why it is the way it is. But I think it 
> doesn't mean that you cannot do something like this:
> 
> #define AUDIT_FILTER_EXCLUDE    AUDIT_FILTER_TYPE

I was originally hoping to do that, but that then causes a build error
on any previous version of audit userspace.

> It's easy then to add a #ifndef to the userspace code so that there is an 
> easy migration. I also do not see any compiler warnings with the above in 
> both /usr/include/linux/audit.h and /usr/include/libaudit.h.

I know it would be easy to fix and going forward userspace could adopt
the kernel one with

	#ifndef AUDIT_FILTER_EXCL
	#define AUDIT_FILTER_EXCL AUDIT_FILTER_TYPE
	#endif

I do have a minor preference for the shorter AUDIT_FILTER_EXCL.

The only place in userspace that AUDIT_FILTER_TYPE is used is grabbing
the value from the kernel to set AUDIT_FILTER_EXCLUDE, and in two bits
of documentation (since that is the published API for libaudit).

Might it make sense to still make AUDIT_FILTER_TYPE available for
libaudit users, but change over the documentation to use one of the
newer forms?

> -Steve
> 
> > In order to
> > not cause userspace compile problems from duplicate definitions and to
> > more accurately and inclusively rename it in the kernel, while providing
> > a migration path for userspace, rename it to AUDIT_FILTER_EXCL.
> > 
> > See: https://github.com/linux-audit/audit-kernel/issues/89
> > 
> > Signed-off-by: Richard Guy Briggs <rgb@redhat.com>
> > ---
> >  include/uapi/linux/audit.h |  3 ++-
> >  kernel/audit.c             |  2 +-
> >  kernel/auditfilter.c       | 10 +++++-----
> >  3 files changed, 8 insertions(+), 7 deletions(-)
> > 
> > diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> > index 04f9bd2..45dd7ef 100644
> > --- a/include/uapi/linux/audit.h
> > +++ b/include/uapi/linux/audit.h
> > @@ -156,8 +156,9 @@
> >  #define AUDIT_FILTER_ENTRY	0x02	/* Apply rule at syscall entry */
> >  #define AUDIT_FILTER_WATCH	0x03	/* Apply rule to file system watches */
> >  #define AUDIT_FILTER_EXIT	0x04	/* Apply rule at syscall exit */
> > -#define AUDIT_FILTER_TYPE	0x05	/* Apply rule at audit_log_start */
> > +#define AUDIT_FILTER_EXCL	0x05	/* Apply rule at audit_log_start */
> >  #define AUDIT_FILTER_FS		0x06	/* Apply rule at __audit_inode_child */
> > +#define AUDIT_FILTER_TYPE	AUDIT_FILTER_EXCL /* obsolete misleading naming
> > */
> > 
> >  #define AUDIT_NR_FILTERS	7
> > 
> > diff --git a/kernel/audit.c b/kernel/audit.c
> > index 3a18e59..089cede 100644
> > --- a/kernel/audit.c
> > +++ b/kernel/audit.c
> > @@ -1754,7 +1754,7 @@ struct audit_buffer *audit_log_start(struct
> > audit_context *ctx, gfp_t gfp_mask, if (audit_initialized !=
> > AUDIT_INITIALIZED)
> >  		return NULL;
> > 
> > -	if (unlikely(!audit_filter(type, AUDIT_FILTER_TYPE)))
> > +	if (unlikely(!audit_filter(type, AUDIT_FILTER_EXCL)))
> >  		return NULL;
> > 
> >  	/* NOTE: don't ever fail/sleep on these two conditions:
> > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> > index eaa3201..f17a42f5 100644
> > --- a/kernel/auditfilter.c
> > +++ b/kernel/auditfilter.c
> > @@ -264,7 +264,7 @@ static inline struct audit_entry
> > *audit_to_entry_common(struct audit_rule_data * case AUDIT_FILTER_TASK:
> >  #endif
> >  	case AUDIT_FILTER_USER:
> > -	case AUDIT_FILTER_TYPE:
> > +	case AUDIT_FILTER_EXCL:
> >  	case AUDIT_FILTER_FS:
> >  		;
> >  	}
> > @@ -337,7 +337,7 @@ static int audit_field_valid(struct audit_entry *entry,
> > struct audit_field *f) {
> >  	switch(f->type) {
> >  	case AUDIT_MSGTYPE:
> > -		if (entry->rule.listnr != AUDIT_FILTER_TYPE &&
> > +		if (entry->rule.listnr != AUDIT_FILTER_EXCL &&
> >  		    entry->rule.listnr != AUDIT_FILTER_USER)
> >  			return -EINVAL;
> >  		break;
> > @@ -931,7 +931,7 @@ static inline int audit_add_rule(struct audit_entry
> > *entry) /* If any of these, don't count towards total */
> >  	switch(entry->rule.listnr) {
> >  	case AUDIT_FILTER_USER:
> > -	case AUDIT_FILTER_TYPE:
> > +	case AUDIT_FILTER_EXCL:
> >  	case AUDIT_FILTER_FS:
> >  		dont_count = 1;
> >  	}
> > @@ -1013,7 +1013,7 @@ int audit_del_rule(struct audit_entry *entry)
> >  	/* If any of these, don't count towards total */
> >  	switch(entry->rule.listnr) {
> >  	case AUDIT_FILTER_USER:
> > -	case AUDIT_FILTER_TYPE:
> > +	case AUDIT_FILTER_EXCL:
> >  	case AUDIT_FILTER_FS:
> >  		dont_count = 1;
> >  	}
> > @@ -1369,7 +1369,7 @@ int audit_filter(int msgtype, unsigned int listtype)
> >  				break;
> >  		}
> >  		if (result > 0) {
> > -			if (e->rule.action == AUDIT_NEVER || listtype == AUDIT_FILTER_TYPE)
> > +			if (e->rule.action == AUDIT_NEVER || listtype == AUDIT_FILTER_EXCL)
> >  				ret = 0;
> >  			break;
> >  		}
> 
> 
> 
> 

- 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

Re: [RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL

[Steve Grubb]

On Thursday, May 31, 2018 6:21:20 PM EDT Richard Guy Briggs wrote:
> On 2018-05-31 17:29, Steve Grubb wrote:
> > On Thursday, May 31, 2018 4:23:09 PM EDT Richard Guy Briggs wrote:
> > > The AUDIT_FILTER_TYPE name is vague and misleading due to not
> > > describing
> > > where or when the filter is applied and obsolete due to its available
> > > filter fields having been expanded.
> > > 
> > > Userspace has already renamed it from AUDIT_FILTER_TYPE to
> > > AUDIT_FILTER_EXCLUDE without checking if it already exists.
> > 
> > Historically speaking, this is not why it is the way it is. But I think
> > it
> > doesn't mean that you cannot do something like this:
> > 
> > #define AUDIT_FILTER_EXCLUDE    AUDIT_FILTER_TYPE
> 
> I was originally hoping to do that, but that then causes a build error
> on any previous version of audit userspace.

I cannot reproduce this. What error did you get? What version of gcc?

Thanks,
-Steve

Re: [RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL

[Richard Guy Briggs]

On 2018-06-01 12:55, Steve Grubb wrote:
> On Thursday, May 31, 2018 6:21:20 PM EDT Richard Guy Briggs wrote:
> > On 2018-05-31 17:29, Steve Grubb wrote:
> > > On Thursday, May 31, 2018 4:23:09 PM EDT Richard Guy Briggs wrote:
> > > > The AUDIT_FILTER_TYPE name is vague and misleading due to not
> > > > describing
> > > > where or when the filter is applied and obsolete due to its available
> > > > filter fields having been expanded.
> > > > 
> > > > Userspace has already renamed it from AUDIT_FILTER_TYPE to
> > > > AUDIT_FILTER_EXCLUDE without checking if it already exists.
> > > 
> > > Historically speaking, this is not why it is the way it is. But I think
> > > it
> > > doesn't mean that you cannot do something like this:
> > > 
> > > #define AUDIT_FILTER_EXCLUDE    AUDIT_FILTER_TYPE
> > 
> > I was originally hoping to do that, but that then causes a build error
> > on any previous version of audit userspace.
> 
> I cannot reproduce this. What error did you get? What version of gcc?

I didn't even try to compile it since I'd predicted that there would be
a symbol definition conflict.

How did you not get a conflict with that definition also in the kernel
header?

> -Steve

- 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

Re: [RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL

[Steve Grubb]

On Friday, June 1, 2018 1:58:34 PM EDT Richard Guy Briggs wrote:
> On 2018-06-01 12:55, Steve Grubb wrote:
> > On Thursday, May 31, 2018 6:21:20 PM EDT Richard Guy Briggs wrote:
> > > On 2018-05-31 17:29, Steve Grubb wrote:
> > > > On Thursday, May 31, 2018 4:23:09 PM EDT Richard Guy Briggs wrote:
> > > > > The AUDIT_FILTER_TYPE name is vague and misleading due to not
> > > > > describing
> > > > > where or when the filter is applied and obsolete due to its
> > > > > available
> > > > > filter fields having been expanded.
> > > > > 
> > > > > Userspace has already renamed it from AUDIT_FILTER_TYPE to
> > > > > AUDIT_FILTER_EXCLUDE without checking if it already exists.
> > > > 
> > > > Historically speaking, this is not why it is the way it is. But I
> > > > think
> > > > it
> > > > doesn't mean that you cannot do something like this:
> > > > 
> > > > #define AUDIT_FILTER_EXCLUDE    AUDIT_FILTER_TYPE
> > > 
> > > I was originally hoping to do that, but that then causes a build error
> > > on any previous version of audit userspace.
> > 
> > I cannot reproduce this. What error did you get? What version of gcc?
> 
> I didn't even try to compile it since I'd predicted that there would be
> a symbol definition conflict.
> 
> How did you not get a conflict with that definition also in the kernel
> header?

It's an identical definition. That's OK. Changes to a definition is last one 
wins - but you get a warning not an error.

-Steve

Re: [RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL

[Richard Guy Briggs]

On 2018-06-01 15:03, Steve Grubb wrote:
> On Friday, June 1, 2018 1:58:34 PM EDT Richard Guy Briggs wrote:
> > On 2018-06-01 12:55, Steve Grubb wrote:
> > > On Thursday, May 31, 2018 6:21:20 PM EDT Richard Guy Briggs wrote:
> > > > On 2018-05-31 17:29, Steve Grubb wrote:
> > > > > On Thursday, May 31, 2018 4:23:09 PM EDT Richard Guy Briggs wrote:
> > > > > > The AUDIT_FILTER_TYPE name is vague and misleading due to not
> > > > > > describing
> > > > > > where or when the filter is applied and obsolete due to its
> > > > > > available
> > > > > > filter fields having been expanded.
> > > > > > 
> > > > > > Userspace has already renamed it from AUDIT_FILTER_TYPE to
> > > > > > AUDIT_FILTER_EXCLUDE without checking if it already exists.
> > > > > 
> > > > > Historically speaking, this is not why it is the way it is. But I
> > > > > think
> > > > > it
> > > > > doesn't mean that you cannot do something like this:
> > > > > 
> > > > > #define AUDIT_FILTER_EXCLUDE    AUDIT_FILTER_TYPE
> > > > 
> > > > I was originally hoping to do that, but that then causes a build error
> > > > on any previous version of audit userspace.
> > > 
> > > I cannot reproduce this. What error did you get? What version of gcc?
> > 
> > I didn't even try to compile it since I'd predicted that there would be
> > a symbol definition conflict.
> > 
> > How did you not get a conflict with that definition also in the kernel
> > header?
> 
> It's an identical definition. That's OK. Changes to a definition is last one 
> wins - but you get a warning not an error.

Do any distros compile with -Werror?

> -Steve

- 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

Re: [RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL

[Steve Grubb]

On Friday, June 1, 2018 3:12:15 PM EDT Richard Guy Briggs wrote:
> On 2018-06-01 15:03, Steve Grubb wrote:
> > On Friday, June 1, 2018 1:58:34 PM EDT Richard Guy Briggs wrote:
> > > On 2018-06-01 12:55, Steve Grubb wrote:
> > > > On Thursday, May 31, 2018 6:21:20 PM EDT Richard Guy Briggs wrote:
> > > > > On 2018-05-31 17:29, Steve Grubb wrote:
> > > > > > On Thursday, May 31, 2018 4:23:09 PM EDT Richard Guy Briggs 
wrote:
> > > > > > > The AUDIT_FILTER_TYPE name is vague and misleading due to not
> > > > > > > describing
> > > > > > > where or when the filter is applied and obsolete due to its
> > > > > > > available
> > > > > > > filter fields having been expanded.
> > > > > > > 
> > > > > > > Userspace has already renamed it from AUDIT_FILTER_TYPE to
> > > > > > > AUDIT_FILTER_EXCLUDE without checking if it already exists.
> > > > > > 
> > > > > > Historically speaking, this is not why it is the way it is. But I
> > > > > > think
> > > > > > it
> > > > > > doesn't mean that you cannot do something like this:
> > > > > > 
> > > > > > #define AUDIT_FILTER_EXCLUDE    AUDIT_FILTER_TYPE
> > > > > 
> > > > > I was originally hoping to do that, but that then causes a build
> > > > > error
> > > > > on any previous version of audit userspace.
> > > > 
> > > > I cannot reproduce this. What error did you get? What version of gcc?
> > > 
> > > I didn't even try to compile it since I'd predicted that there would be
> > > a symbol definition conflict.
> > > 
> > > How did you not get a conflict with that definition also in the kernel
> > > header?
> > 
> > It's an identical definition. That's OK. Changes to a definition is last
> > one wins - but you get a warning not an error.
> 
> Do any distros compile with -Werror?

Audit itself can't be compiled with -Werror as there are lots of warnings 
about using string functions with unsigned chars. However, libaudit.h is used 
in 20 or so packages and there is a chance one may have -Werror. But I think 
its unlikely based on a recent project which involved looking over static 
analysis results for a large chunk of the Fedora 27 repo. Out of 4730 source 
packages, 84 had no compiler warnings. So, I'd say its next to impossible for 
any distribution to make -Werror a blanket policy.

-Steve

Re: [RFC PATCH ghak89 V1] audit: rename FILTER_TYPE to FILTER_EXCL

[Richard Guy Briggs]

On 2018-06-01 15:37, Steve Grubb wrote:
> On Friday, June 1, 2018 3:12:15 PM EDT Richard Guy Briggs wrote:
> > On 2018-06-01 15:03, Steve Grubb wrote:
> > > On Friday, June 1, 2018 1:58:34 PM EDT Richard Guy Briggs wrote:
> > > > On 2018-06-01 12:55, Steve Grubb wrote:
> > > > > On Thursday, May 31, 2018 6:21:20 PM EDT Richard Guy Briggs wrote:
> > > > > > On 2018-05-31 17:29, Steve Grubb wrote:
> > > > > > > On Thursday, May 31, 2018 4:23:09 PM EDT Richard Guy Briggs 
> wrote:
> > > > > > > > The AUDIT_FILTER_TYPE name is vague and misleading due to not
> > > > > > > > describing
> > > > > > > > where or when the filter is applied and obsolete due to its
> > > > > > > > available
> > > > > > > > filter fields having been expanded.
> > > > > > > > 
> > > > > > > > Userspace has already renamed it from AUDIT_FILTER_TYPE to
> > > > > > > > AUDIT_FILTER_EXCLUDE without checking if it already exists.
> > > > > > > 
> > > > > > > Historically speaking, this is not why it is the way it is. But I
> > > > > > > think
> > > > > > > it
> > > > > > > doesn't mean that you cannot do something like this:
> > > > > > > 
> > > > > > > #define AUDIT_FILTER_EXCLUDE    AUDIT_FILTER_TYPE
> > > > > > 
> > > > > > I was originally hoping to do that, but that then causes a build
> > > > > > error
> > > > > > on any previous version of audit userspace.
> > > > > 
> > > > > I cannot reproduce this. What error did you get? What version of gcc?
> > > > 
> > > > I didn't even try to compile it since I'd predicted that there would be
> > > > a symbol definition conflict.
> > > > 
> > > > How did you not get a conflict with that definition also in the kernel
> > > > header?
> > > 
> > > It's an identical definition. That's OK. Changes to a definition is last
> > > one wins - but you get a warning not an error.
> > 
> > Do any distros compile with -Werror?
> 
> Audit itself can't be compiled with -Werror as there are lots of warnings 
> about using string functions with unsigned chars. However, libaudit.h is used 
> in 20 or so packages and there is a chance one may have -Werror. But I think 
> its unlikely based on a recent project which involved looking over static 
> analysis results for a large chunk of the Fedora 27 repo. Out of 4730 source 
> packages, 84 had no compiler warnings. So, I'd say its next to impossible for 
> any distribution to make -Werror a blanket policy.

Ok, I'll switch my patch to match your definition.

Is there any plan to migrate the documentation to match?

> -Steve

- 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