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

[Steve Grubb <sgrubb@redhat.com>]

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;
>  		}