Re: null pointer dereference regression in 5.7

2020-07-22 Thread Paul Moore
On Tue, Jul 21, 2020 at 10:01 PM Richard Guy Briggs  wrote:
> On 2020-07-21 18:45, Paul Moore wrote:
> > On Tue, Jul 21, 2020 at 6:30 PM Paul Moore  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.

I would expect that it would resolve the problem being reported, which
is good, but I'm not sure it is a general solution to the problem.  I
suspect there is bigger problem of context->pwd not always having a
"safe" value when the task exits or the syscall returns to userspace.

> > 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.

Yes, the issue is with context->pwd, although I suppose other fields
could also be suspect.

> > 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.

Do you have a rough idea as to how long it would take to chase down
all the code paths?  I'm asking not to rush you, but to figure out if
we should revert the patch now to resolve the problem and restore it
later once we are confident there are no additional issues lurking.

-- 
paul moore
www.paul-moore.com

--
Linux-audit mailing list
Linux-audit@redhat.com
https://www.redhat.com/mailman/listinfo/linux-audit



Re: null pointer dereference regression in 5.7

2020-07-22 Thread Dominick Grift



On 7/22/20 9:47 PM, Richard Guy Briggs wrote:
> On 2020-07-18 20:56, Dominick Grift wrote:
>> On 7/18/20 8:40 PM, bauen1 wrote:
>>> Hi,
>>> After upgrading from linux 5.6 to 5.7 on my debian machines with selinux 
>>> I've started seeing this null pointer dereference in the audit system. I've 
>>> included shortened logs for 5.6 without the error and from 5.7 with the 
>>> error from my laptop. I've also seen it happen in a VM and a server, but 
>>> don't have the logs anymore. Grift was able to reproduced (presumably) the 
>>> same issue on fedora with 5.8-rc4.
>>>
>>> Steps to reproduce:
>>> Write an selinux policy with a domain for systemd-user-runtime-dir and 
>>> audit all permissions of the dir class. E.g. `(auditallow 
>>> systemd_user_runtime_dir_t all_types (dir (all)))`
>>> Switch to permissive mode.
>>> Create a new user and login, log out and wait a few seconds for systemd to 
>>> stop user-runtime-dir@.service
>>
>> This should be a reproducer:
>>
>> echo "(auditallow systemd_logind_t file_type (dir (all)))" > mytest.cil
>> && sudo semodule -i mytest.cil
>> reboot
> 
> Is this recipe complete?  Is permissive mode needed?  Is the user
> create/login/logout needed?

Are you saying you can't reproduce it?

It *should* be complete yes. with kernel 5.7/5.8 it should oops when you
reboot.

I will admit though that I adjusted the reproducer a little bit in an
attempt to make it fit fedora.

So if it doesnt oops for you and if you use 5.7/5.8 then maybe the
reproducer got mangled in the conversion.



> 
>>> I believe this issue was made visible by 
>>> 1320a4052ea11eb2879eb7361da15a106a780972.
>>> Now a AUDIT_PATH event is also generated by default and 
>>> systemd-user-runtime-dir is making syscalls that audit_log_name can't 
>>> handle.
>>>
>>> I hope this is enough info to find the root cause.
>>> - bauen1
>>>
>>> Log without crash (5.6):
>>>
>>> Jul 18 14:26:36 jh-mba kernel: Linux version 5.6.0-2-amd64 
>>> (debian-ker...@lists.debian.org) (gcc version 9.3.0 (Debian 9.3.0-13)) #1 
>>> SMP Debian 5.6.14-2 (2020-06-09)
>>> Jul 18 14:27:53 jh-mba audit[1]: SERVICE_STOP pid=1 uid=0 auid=4294967295 
>>> ses=4294967295 subj=system_u:system_r:init_t:s0 msg='unit=user@1001 
>>> comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? 
>>> res=success'
>>> Jul 18 14:27:53 jh-mba systemd[1]: Stopping User Runtime Directory 
>>> /run/user/1001...
>>> Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { read } for  
>>> pid=3178 comm="systemd-user-ru" name="dconf" dev="tmpfs" ino=41325 
>>> scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
>>> tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=dir permissive=1
>>> Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { open } for  
>>> pid=3178 comm="systemd-user-ru" path="/run/user/1001/dconf" dev="tmpfs" 
>>> ino=41325 scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
>>> tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=dir permissive=1
>>> Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { getattr } for  
>>> pid=3178 comm="systemd-user-ru" path="/run/user/1001/dconf" dev="tmpfs" 
>>> ino=41325 scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
>>> tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=dir permissive=1
>>> Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { search } for  
>>> pid=3178 comm="systemd-user-ru" name="dconf" dev="tmpfs" ino=41325 
>>> scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
>>> tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=dir permissive=1
>>> Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { write } for  
>>> pid=3178 comm="systemd-user-ru" name="dconf" dev="tmpfs" ino=41325 
>>> scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
>>> tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=dir permissive=1
>>> Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { remove_name } for  
>>> pid=3178 comm="systemd-user-ru" name="user" dev="tmpfs" ino=41326 
>>> scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
>>> tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=dir permissive=1
>>> Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { unlink } for  
>>> pid=3178 comm="systemd-user-ru" name="user" dev="tmpfs" ino=41326 
>>> scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
>>> tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=file permissive=1
>>> Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { rmdir } for  
>>> pid=3178 comm="systemd-user-ru" name="dconf" dev="tmpfs" ino=41325 
>>> scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
>>> tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=dir permissive=1
>>> Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { read } for  
>>> pid=3178 comm="systemd-user-ru" name="gvfs" dev="tmpfs" ino=42315 
>>> scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
>>> tcontext=user_u:object_r:user_tmp_t:s0 tclass=dir permissive=1
>>> Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { open } for 

Re: null pointer dereference regression in 5.7

2020-07-22 Thread Richard Guy Briggs
On 2020-07-18 20:56, Dominick Grift wrote:
> On 7/18/20 8:40 PM, bauen1 wrote:
> > Hi,
> > After upgrading from linux 5.6 to 5.7 on my debian machines with selinux 
> > I've started seeing this null pointer dereference in the audit system. I've 
> > included shortened logs for 5.6 without the error and from 5.7 with the 
> > error from my laptop. I've also seen it happen in a VM and a server, but 
> > don't have the logs anymore. Grift was able to reproduced (presumably) the 
> > same issue on fedora with 5.8-rc4.
> > 
> > Steps to reproduce:
> > Write an selinux policy with a domain for systemd-user-runtime-dir and 
> > audit all permissions of the dir class. E.g. `(auditallow 
> > systemd_user_runtime_dir_t all_types (dir (all)))`
> > Switch to permissive mode.
> > Create a new user and login, log out and wait a few seconds for systemd to 
> > stop user-runtime-dir@.service
> 
> This should be a reproducer:
> 
> echo "(auditallow systemd_logind_t file_type (dir (all)))" > mytest.cil
> && sudo semodule -i mytest.cil
> reboot

Is this recipe complete?  Is permissive mode needed?  Is the user
create/login/logout needed?

> > I believe this issue was made visible by 
> > 1320a4052ea11eb2879eb7361da15a106a780972.
> > Now a AUDIT_PATH event is also generated by default and 
> > systemd-user-runtime-dir is making syscalls that audit_log_name can't 
> > handle.
> > 
> > I hope this is enough info to find the root cause.
> > - bauen1
> > 
> > Log without crash (5.6):
> > 
> > Jul 18 14:26:36 jh-mba kernel: Linux version 5.6.0-2-amd64 
> > (debian-ker...@lists.debian.org) (gcc version 9.3.0 (Debian 9.3.0-13)) #1 
> > SMP Debian 5.6.14-2 (2020-06-09)
> > Jul 18 14:27:53 jh-mba audit[1]: SERVICE_STOP pid=1 uid=0 auid=4294967295 
> > ses=4294967295 subj=system_u:system_r:init_t:s0 msg='unit=user@1001 
> > comm="systemd" exe="/usr/lib/systemd/systemd" hostname=? addr=? terminal=? 
> > res=success'
> > Jul 18 14:27:53 jh-mba systemd[1]: Stopping User Runtime Directory 
> > /run/user/1001...
> > Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { read } for  
> > pid=3178 comm="systemd-user-ru" name="dconf" dev="tmpfs" ino=41325 
> > scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
> > tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=dir permissive=1
> > Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { open } for  
> > pid=3178 comm="systemd-user-ru" path="/run/user/1001/dconf" dev="tmpfs" 
> > ino=41325 scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
> > tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=dir permissive=1
> > Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { getattr } for  
> > pid=3178 comm="systemd-user-ru" path="/run/user/1001/dconf" dev="tmpfs" 
> > ino=41325 scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
> > tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=dir permissive=1
> > Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { search } for  
> > pid=3178 comm="systemd-user-ru" name="dconf" dev="tmpfs" ino=41325 
> > scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
> > tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=dir permissive=1
> > Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { write } for  
> > pid=3178 comm="systemd-user-ru" name="dconf" dev="tmpfs" ino=41325 
> > scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
> > tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=dir permissive=1
> > Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { remove_name } for  
> > pid=3178 comm="systemd-user-ru" name="user" dev="tmpfs" ino=41326 
> > scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
> > tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=dir permissive=1
> > Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { unlink } for  
> > pid=3178 comm="systemd-user-ru" name="user" dev="tmpfs" ino=41326 
> > scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
> > tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=file permissive=1
> > Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { rmdir } for  
> > pid=3178 comm="systemd-user-ru" name="dconf" dev="tmpfs" ino=41325 
> > scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
> > tcontext=user_u:object_r:gconf_tmp_t:s0 tclass=dir permissive=1
> > Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { read } for  
> > pid=3178 comm="systemd-user-ru" name="gvfs" dev="tmpfs" ino=42315 
> > scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
> > tcontext=user_u:object_r:user_tmp_t:s0 tclass=dir permissive=1
> > Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { open } for  
> > pid=3178 comm="systemd-user-ru" path="/run/user/1001/gvfs" dev="tmpfs" 
> > ino=42315 scontext=system_u:system_r:systemd_user_runtime_dir_t:s0 
> > tcontext=user_u:object_r:user_tmp_t:s0 tclass=dir permissive=1
> > Jul 18 14:27:53 jh-mba audit[3178]: AVC avc:  denied  { getattr } for  
> > pid=3178 comm="systemd-user-ru" path="/run/user/1001/gvfs" dev="tmpfs" 
> > ino=42315