On 14/09/24, Richard Guy Briggs wrote:
> A regression was caused by commit 780a7654cee8:
>        audit: Make testing for a valid loginuid explicit.
> (which in turn attempted to fix a regression caused by e1760bd)

Eric (Paris),  does tagging this field in the type member with a high
bit work for you?  It only has to be filtered out for
audit_filter_user_rules() and audit_filter_rules() to avoid changing the
normal semantics of that field.  I'd rather do this than add a new
member to the field struct.

> When audit_krule_to_data() fills in the rules to get a listing, there was a
> missing clause to convert back from AUDIT_LOGINUID_SET to AUDIT_LOGINUID.
> 
> This broke userspace by not returning the same information that was sent and
> expected.
> 
> The rule:
>       auditctl -a exit,never -F auid=-1
> gives:
>       auditctl -l
>               LIST_RULES: exit,never f24=0 syscall=all
> when it should give:
>               LIST_RULES: exit,never auid=-1 (0xffffffff) syscall=all
> 
> Tag it so that it is reported the same way it was set.
> 
> Note: move the field validation call ahead of the mutation code to have it 
> work
> on the original field set.
> 

I intend to add a Cc: stable 3.10-rc1 here.

> Signed-off-by: Richard Guy Briggs <[email protected]>
> ---
>  include/uapi/linux/audit.h |    3 +++
>  kernel/auditfilter.c       |   19 +++++++++++++------
>  kernel/auditsc.c           |    2 +-
>  3 files changed, 17 insertions(+), 7 deletions(-)
> 
> diff --git a/include/uapi/linux/audit.h b/include/uapi/linux/audit.h
> index 4d100c8..860df86 100644
> --- a/include/uapi/linux/audit.h
> +++ b/include/uapi/linux/audit.h
> @@ -274,6 +274,9 @@
>  
>  #define AUDIT_FILTERKEY      210
>  
> +/* Flag to indicate legacy AUDIT_LOGINUID unset usage */
> +#define AUDIT_LOGINUID_LEGACY                0x80000000
> +
>  #define AUDIT_NEGATE                 0x80000000
>  
>  /* These are the supported operators.
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 40ed981..39ce3e6 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -438,9 +438,13 @@ static struct audit_entry *audit_data_to_entry(struct 
> audit_rule_data *data,
>               f->type = data->fields[i];
>               f->val = data->values[i];
>  
> +             err = audit_field_valid(entry, f);
> +             if (err)
> +                     goto exit_free;
> +
>               /* Support legacy tests for a valid loginuid */
>               if ((f->type == AUDIT_LOGINUID) && (f->val == AUDIT_UID_UNSET)) 
> {
> -                     f->type = AUDIT_LOGINUID_SET;
> +                     f->type = AUDIT_LOGINUID_SET | AUDIT_LOGINUID_LEGACY;
>                       f->val = 0;
>               }
>  
> @@ -457,10 +461,6 @@ static struct audit_entry *audit_data_to_entry(struct 
> audit_rule_data *data,
>                       rcu_read_unlock();
>               }
>  
> -             err = audit_field_valid(entry, f);
> -             if (err)
> -                     goto exit_free;
> -
>               err = -EINVAL;
>               switch (f->type) {
>               case AUDIT_LOGINUID:
> @@ -630,6 +630,13 @@ static struct audit_rule_data 
> *audit_krule_to_data(struct audit_krule *krule)
>                       data->buflen += data->values[i] =
>                               audit_pack_string(&bufp, krule->filterkey);
>                       break;
> +             case AUDIT_LOGINUID_SET | AUDIT_LOGINUID_LEGACY:
> +                     if (!f->val) {
> +                             data->fields[i] = AUDIT_LOGINUID;
> +                             data->values[i] = AUDIT_UID_UNSET;
> +                             break;
> +                     }
> +                     /* fallthrough if set */
>               default:
>                       data->values[i] = f->val;
>               }
> @@ -1270,7 +1277,7 @@ static int audit_filter_user_rules(struct audit_krule 
> *rule, int type,
>               int result = 0;
>               u32 sid;
>  
> -             switch (f->type) {
> +             switch (f->type & ~AUDIT_LOGINUID_LEGACY) {
>               case AUDIT_PID:
>                       pid = task_pid_nr(current);
>                       result = audit_comparator(pid, f->op, f->val);
> diff --git a/kernel/auditsc.c b/kernel/auditsc.c
> index 8933572..ef25cbc 100644
> --- a/kernel/auditsc.c
> +++ b/kernel/auditsc.c
> @@ -452,7 +452,7 @@ static int audit_filter_rules(struct task_struct *tsk,
>               int result = 0;
>               pid_t pid;
>  
> -             switch (f->type) {
> +             switch (f->type & ~AUDIT_LOGINUID_LEGACY) {
>               case AUDIT_PID:
>                       pid = task_pid_nr(tsk);
>                       result = audit_comparator(pid, f->op, f->val);
> -- 
> 1.7.1
> 

- RGB

--
Richard Guy Briggs <[email protected]>
Senior Software Engineer, Kernel Security, AMER ENG Base Operating Systems, Red 
Hat
Remote, Ottawa, Canada
Voice: +1.647.777.2635, Internal: (81) 32635, Alt: +1.613.693.0684x3545

--
Linux-audit mailing list
[email protected]
https://www.redhat.com/mailman/listinfo/linux-audit

Reply via email to