Re: [PATCH v5] libselinux: use kernel status page by default

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Jul 31, 2020 at 1:26 PM Mike Palmiotto
<mike.palmiotto@xxxxxxxxxxxxxxx> wrote:
>
> Commit bc2a8f418e3b ("libselinux: add selinux_status_* interfaces for
> /selinux/status") introduced the sestatus mechanism, which allows for
> mmap()'ing of the kernel status page as a replacement for avc_netlink.
>
> The mechanism was initially intended for userspace object managers that
> were calculating access decisions within their application and did not
> rely on the libselinux AVC implementation. In order to properly make use
> of sestatus within avc_has_perm(), the status mechanism needs to
> properly set avc internals during status events; else, avc_enforcing is
> never updated upon sestatus changes.
>
> This commit gets rid of the default avc_netlink_open() in
> avc_init_internal(), replacing it with selinux_status_open(). In the
> event that the kernel status page cannot be mapped, the netlink fallback
> will be used. By default, avc_has_perm_noaudit() and
> selinux_check_access() will now attempt to read the kernel status page,
> which removes a system call from two critical code paths.
>
> Since the AVC thread create/stop callbacks were intended to avoid a
> system call in the critical code path, they no longer need to be created
> by default. In the event that the kernel status page is successfully
> mapped, threads will not be created. Threads will still be
> created/stopped for the sestatus fallback codepaths.
>
> Userspace object managers that still need a netlink socket can call
> avc_netlink_acquire_fd() to open and/or obtain one.
>
> Update the manpage to reflect the new avc_netlink_acquire_fd()
> functionality.
>
> Signed-off-by: Mike Palmiotto <mike.palmiotto@xxxxxxxxxxxxxxx>
> ---

Looks good aside from man page comments below.

> diff --git a/libselinux/man/man3/avc_init.3 b/libselinux/man/man3/avc_init.3
> index e26c3be6..656d7e24 100644
> --- a/libselinux/man/man3/avc_init.3
> +++ b/libselinux/man/man3/avc_init.3
> @@ -153,14 +155,74 @@ callback should destroy
>  .IR lock ,
>  freeing any resources associated with it.  The default behavior is not to perform any locking.  Note that undefined behavior may result if threading is used without appropriate locking.
>  .
> -.SH "NETLINK NOTIFICATION"
> -Beginning with version 2.6.4, the Linux kernel supports SELinux status change notification via netlink.  Two message types are currently implemented, indicating changes to the enforcing mode and to the loaded policy in the kernel, respectively.  The userspace AVC listens for these messages and takes the appropriate action, modifying the behavior of
> -.BR avc_has_perm (3)
> -to reflect the current enforcing mode and flushing the cache on receipt of a policy load notification.  Audit messages are produced when netlink notifications are processed.
> +.SH "KERNEL STATUS PAGE"
> +Linux kernel version 2.6.37 supports the SELinux kernel status page, enabling userspace applications to
> +.BR mmap (2)
> +SELinux status state in read-only mode to avoid system calls during the cache hit code path.
> +
> +Status state is stored in the
> +.B selinux_status
> +pointer to the
> +.B selinux_status_t
> +struct.
> +
> +.RS
> +.ta 4n 10n 24n
> +.nf
> +
> +struct selinux_status_t
> +{
> +       uint32_t        version;        /* version number of this structure */
> +       uint32_t        sequence;       /* sequence number of seqlock logic */
> +       uint32_t        enforcing;      /* current setting of enforcing mode */
> +       uint32_t        policyload;     /* times of policy reloaded */
> +       uint32_t        deny_unknown;   /* current setting of deny_unknown */
> +       /* version > 0 support above status */
> +} __attribute((packed));
> +.fi
> +.ta
> +.RE

Unless this pointer and structure are exposed to users of the shared
library, I don't know why we'd document them in the man page.  The
struct definition is only needed for direct users of selinuxfs that
don't go through libselinux (if any), and that would logically go into
a selinuxfs man page if we ever created one of those.  You can refer
to the SELinux status page as a general mechanism similar to the
SELinux netlink socket without exposing library internals here.

> +Two status types are currently implemented.
> +.B avc_process_setenforce
> +will process changes to the enforcing mode by changing the effective enforcing state used with the AVC, and
> +.B avc_process_policyload
> +will process changes to the kernel loaded policy by flushing the cache.
> +.

Likewise these are libselinux-internal functions not exposed to users
of the library and hence wouldn't appear in the man page.

> +.SH "NETLINK NOTIFICATION"
> +In the event that the kernel status page is not successfully
> +.BR mmap (3)'ed
> +into
> +.B selinux_status,

Can omit "into selinux_status".

> +the AVC will default to the netlink fallback mechanism.
> +
> +By default,
> +.BR avc_open (3)
> +does not set threading or locking callbacks. In this case, the userspace AVC checks for new netlink messages at the start of each permission query. If threading and locking callbacks are passed to
> +.BR avc_init (),
> +a dedicated thread will be started to listen on the netlink socket.  This may increase performance in the absense of

spelling - absence

> +.B selinux_status

the selinux status page

> +and will ensure that log messages are generated immediately rather than at the time of the next permission query.
>  .
>  .SH "RETURN VALUE"
>  Functions with a return value return zero on success.  On error, \-1 is returned and

Does the avc_open.3 man page need updating too?




[Index of Archives]     [Selinux Refpolicy]     [Linux SGX]     [Fedora Users]     [Fedora Desktop]     [Yosemite Photos]     [Yosemite Camping]     [Yosemite Campsites]     [KDE Users]     [Gnome Users]

  Powered by Linux