On Fri, Oct 7, 2022 at 4:54 AM Petr Lautrbach <plautrba@xxxxxxxxxx> wrote:
>
> Hi,
>
>
> Commit 05bdc03130d74 ("libselinux: use kernel status page by default") changed
> selinux_status_updated() so that it calls avc_process_policyload() and
> avc_process_setenforce() and both functions call avc_log() and avc_log() logs to
> stderr by default. So when a process like `rpm` checks whether there was a
> change, it gets output on stderr which previously wasn't there.
>
>
> Before this change:
> >>> from selinux import *
> >>> selinux_status_open(0);
> 0
> >>>
> >>> selinux_status_updated();
> 0
> >>> selinux_mkload_policy(0);
> 0
> >>> selinux_status_updated();
> 1
>
> Current version:
> >>> from selinux import *
> elinux_status_updated();
> selinux_mkload_policy(0);
> selinux_status_updated();
> >>> selinux_status_open(0);
> 0
> >>> selinux_status_updated();
> 0
> >>> selinux_mkload_policy(0);
> 0
> >>> selinux_status_updated();
> uavc: op=load_policy lsm=selinux seqno=2 res=11
>
>
> The calling process could set its callback but it seems unnecessarily
> complicated just for selinux_status_updated() which is supposed to check whether
> something has changed or not. Also processing events in this function seems to
> be unnecessary.
>
> It looks like the reason for the new code added to selinux_status_updated() is
> that there were several avc_netlink_check_nb() calls replaced by
> selinux_status_updated(). Given the problem described above, I don't think it's
> correct and I would like to change selinux_status_updated() back and use another
> mechanism that would help with the replacement.
>
>
> So what do you think about it?
The goal was to switch the AVC and all of its users (e.g.
selinux_check_access) over to using the much more efficient SELinux
kernel status page mechanism for setenforce and policy load
notifications on newer kernels instead of the SELinux netlink
mechanism (which imposed extra syscall overhead on the critical path).
Understand your concern but unsure as to whether we can just change
selinux_status_updated() back now.
Would require an audit of all users of selinux_status_updated(), both
direct and indirect, to ensure that none of them are relying on this
behavior. We can obviously fix the callers within libselinux but
addressing external callers is more problematic and is arguably an ABI
change. Would need to look at all users of the AVC,
selinux_check_access(), etc.
This change happened 2 years ago so I have to wonder why it is only
coming up now?
