Messages in this thread | | | From | (Eric W. Biederman) | Date | Wed, 10 Apr 2013 11:02:43 -0700 | Subject | Re: [PATCH] [BZ905179] audit: omit check for uid and gid validity in audit rules and data |
| |
Richard Guy Briggs <rgb@redhat.com> writes:
> On Tue, Apr 09, 2013 at 02:39:32AM -0700, Eric W. Biederman wrote:
>> @@ -377,6 +383,12 @@ static struct audit_entry *audit_rule_to_entry(struct audit_rule *rule) >> if (!gid_valid(f->gid)) >> goto exit_free; >> break; >> + case AUDIT_LOGINUID_SET: >> + if ((f->op != Audit_not_equal) && (f->op != Audit_equal)) >> + goto exit_free; >> + if ((f->val != 0) && (f->val != 1)) > > Why the extra comparison to "1"? > > Are you anticipating already a userspace process making a call using the > newof type AUDIT_LOGINUID_SET with a value of 1?
Sorry I missed this question the first time. I am anticipating AUDIT_LOGINUID_SET to return a value of 0 or 1 (a boolean) and so I allow the operations and constants that are valid for a boolean.
In particuluar I allow the opeartions == != and the boolean constants 0 and 1.
>> @@ -1380,6 +1405,10 @@ static int audit_filter_user_rules(struct audit_krule *rule, >> result = audit_uid_comparator(audit_get_loginuid(current), >> f->op, f->uid); >> break; >> + case AUDIT_LOGINUID_SET: >> + result = audit_comparator(audit_loginuid_set(current), >> + f->op, f->val); >> + break; >> case AUDIT_SUBJ_USER: >> case AUDIT_SUBJ_ROLE: >> case AUDIT_SUBJ_TYPE: >> diff --git a/kernel/auditsc.c b/kernel/auditsc.c >> index 3a11d34..27d0a50 100644 >> --- a/kernel/auditsc.c >> +++ b/kernel/auditsc.c >> @@ -750,6 +750,9 @@ static int audit_filter_rules(struct task_struct *tsk, >> if (ctx) >> result = audit_uid_comparator(tsk->loginuid, f->op, f->uid); >> break; >> + case AUDIT_LOGINUID_SET: >> + result = audit_comparator(audit_loginuid_set(tsk), f->op, f->val); >> + break; > > (OT: I assume the "if (ctx)" is wrong in the AUDIT_LOGINUID case > above.)
Good question. I didn't see that when I was preparing my patch.
ctx is not necessary but I think ctx is set when a task is being audited so it may serve a useful function. But I have to admit it that if(ctx) looks like a bug.
Eric
| |