Re: [PATCH] audit: restore AUDIT_LOGINUID unset ABI

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) 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. Signed-off-by: Richard Guy Briggs <rgb(a)redhat.com&gt; --- 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);