Re: null pointer dereference regression in 5.7
On 2020-07-21 18:45, Paul Moore wrote:
On Tue, Jul 21, 2020 at 6:30 PM Paul Moore
<paul(a)paul-moore.com> wrote:
> Richard, you broke it, you bought it :) Did you want to take a closer
> look at this? If you can't let me know. Based on a quick look, my
> gut feeling is that either context->pwd is never set properly or it is
> getting free'd prematurely; I'm highly suspicious of the latter but
> the former seems like it might be a reasonable place to start.
Actually, yes, I'm pretty certain the problem is that context->pwd is
never set in this case.
Does the ghak96 upstream patch in audit/next on 5.8-rc1 fix it?
d7481b24b816 ("audit: issue CWD record to accompany LSM_AUDIT_DATA_* records")
The avc is generated by common_lsm_audit() which calls
dump_common_audit_data() that now calls audit_getcwd() on the 5
LSM_AUDIT_DATA_* types that deal with paths.
Normally context->pwd would be set by a call to
audit_getname()/__audit_getname(), but if there audit context is a
dummy context, that is skipped and context->pwd is never set.
Normally that is fine, expect with Richard's patch if the kernel
explicitly calls audit_log_start() we mark the context as ... not a
dummy? smart? I'm not sure of the right term here ... which then
triggers all the usual logging one would expect. In this particular
case, a SELinux AVC, the audit_log_start() happens *after* the
pathname has been resolved and the audit_getname() calls are made;
thus in this case context->pwd is not valid when the normal audit
logging takes place on exit and things explode in predictable fashion.
The first two AVCs that were accompanied by syscalls had "items=0" but
the one that blew up had "items=2" so it appears the paths were already
present in the context, but missing the pwd.
Unfortunately, it is beginning to look like 1320a4052ea1
("audit:
trigger accompanying records when no rules present") may be more
dangerous than initially thought. I'm borderline tempted to just
revert this patch, but I'll leave this open for discussion ...
Richard, I think you need to go through the code and audit all of the
functions that store data in an audit context that are skipped when
there is a dummy context to see which fields are potentially unset,
and then look at all the end of task/syscall code to make sure the
necessary set/unset checks are in place.
Auditing all the callers is not a small task, but I agree it may be
necessary.
This should be a priority.
paul moore
- RGB
--
Richard Guy Briggs <rgb(a)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