From: Mike Palmiotto <mike.palmiotto@crunchydata.com>
To: SElinux list <selinux@vger.kernel.org>
Subject: Re: [PATCH v3] libselinux: use kernel status page by default
Date: Thu, 23 Jul 2020 18:41:18 -0400 [thread overview]
Message-ID: <CAMN686H-0KVjM7KpMuwMP9J1vOd00oZ=9agm_nEvUUP8fMfVJg@mail.gmail.com> (raw)
In-Reply-To: <20200723223827.16027-1-mike.palmiotto@crunchydata.com>
On Thu, Jul 23, 2020 at 6:38 PM Mike Palmiotto
<mike.palmiotto@crunchydata.com> 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 use by userspace object
> managers which 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 introduces a new selinux_status_loop() function, which
> replaces the default netlink-equivalent, avc_netlink_loop(). The
> function watches the kernel status page until an error occurs, at which
> point it will exit the thread. In the event that the status page cannot
> be opened (on avc_open), the thread will continue to function as before
> by using a fallback netlink socket.
>
> This allows us to replace the call to avc_netlink_open() in
> avc_init_internal() with a call to selinux_status_open() and remove the
> avc_netlink_check_nb() call from the critical code path in
> avc_has_perm_noaudit(), as well as selinux_check_access().
>
> Userspace object managers who still need a netlink socket can call
> avc_netlink_acquire_fd() to open open and/or obtain one.
>
> Update the manpage to reflect the new avc_netlink_acquire_fd()
> functionality.
>
> Signed-off-by: Mike Palmiotto <mike.palmiotto@crunchydata.com>
> ---
> Testing:
> - dbus-daemon v1.12.8 on RHEL8.2
> - dbus-broker v22 on F32
>
> Changelog:
> V2:
> - Added selinux_status_loop function for watcher threads.
> - Replaced avc_netlink_open with selinux_status_open.
> - Moved avc_netlink_open into avc_netlink_acquire_fd.
> - Replaced avc_netlink_check_nb() call in selinux_check_access with sestatus
> equivalent.
> - Updated manpage and mapfile.
>
> V3:
> - Made selinux_status_loop an internal function and got rid of manpage/mapfile
> changes.
> - Got rid of superfluous selinux_status_close() in selinux_status_loop().
> - Got rid of avc_enforcing modification in selinux_status_getenforce().
> - Some style fixes.
> - Updated commit message subject line/details.
>
> libselinux/man/man3/avc_netlink_loop.3 | 8 +++
> libselinux/src/avc.c | 14 ++---
> libselinux/src/avc_internal.c | 82 ++++++++++++++++++--------
> libselinux/src/avc_internal.h | 7 +++
> libselinux/src/checkAccess.c | 2 +-
> libselinux/src/libselinux.map | 1 +
> libselinux/src/sestatus.c | 27 +++++++++
> 7 files changed, 107 insertions(+), 34 deletions(-)
>
> diff --git a/libselinux/man/man3/avc_netlink_loop.3 b/libselinux/man/man3/avc_netlink_loop.3
> index c8268a12..f03d7813 100644
> --- a/libselinux/man/man3/avc_netlink_loop.3
> +++ b/libselinux/man/man3/avc_netlink_loop.3
> @@ -54,6 +54,11 @@ closes the netlink socket. This function is called automatically by
> returns the netlink socket descriptor number and informs the userspace AVC
> not to check the socket descriptor automatically on calls to
> .BR avc_has_perm (3).
> +If no such socket descriptor exists,
> +.BR avc_netlink_acquire_fd (3)
> +will first call
> +.BR avc_netlink_open (3)
> +and then return the resulting fd.
>
> .BR avc_netlink_release_fd ()
> returns control of the netlink socket to the userspace AVC, re-enabling
> @@ -78,6 +83,9 @@ with a return value return zero on success. On error, \-1 is returned and
> .I errno
> is set appropriately.
> .
> +.SH "AUTHOR"
> +Originally KaiGai Kohei. Updated by Mike Palmiotto <mike.palmiotto@crunchydata.com>
> +.
> .SH "SEE ALSO"
> .BR avc_open (3),
> .BR selinux_set_callback (3),
> diff --git a/libselinux/src/avc.c b/libselinux/src/avc.c
> index b4648b2d..5f3b00cf 100644
> --- a/libselinux/src/avc.c
> +++ b/libselinux/src/avc.c
> @@ -50,7 +50,7 @@ struct avc_callback_node {
> struct avc_callback_node *next;
> };
>
> -static void *avc_netlink_thread = NULL;
> +static void *avc_status_thread = NULL;
> static void *avc_lock = NULL;
> static void *avc_log_lock = NULL;
> static struct avc_node *avc_node_freelist = NULL;
> @@ -215,15 +215,15 @@ static int avc_init_internal(const char *prefix,
> avc_enforcing = rc;
> }
>
> - rc = avc_netlink_open(0);
> + rc = selinux_status_open(1);
> if (rc < 0) {
> avc_log(SELINUX_ERROR,
> - "%s: can't open netlink socket: %d (%s)\n",
> + "%s: could not open selinux status page: %d (%s)\n",
> avc_prefix, errno, strerror(errno));
> goto out;
> }
> if (avc_using_threads) {
> - avc_netlink_thread = avc_create_thread(&avc_netlink_loop);
> + avc_status_thread = avc_create_thread(&selinux_status_loop);
> avc_netlink_trouble = 0;
> }
> avc_running = 1;
> @@ -558,8 +558,8 @@ void avc_destroy(void)
> avc_get_lock(avc_lock);
>
> if (avc_using_threads)
> - avc_stop_thread(avc_netlink_thread);
> - avc_netlink_close();
> + avc_stop_thread(avc_status_thread);
> + selinux_status_close();
>
> for (i = 0; i < AVC_CACHE_SLOTS; i++) {
> node = avc_cache.slots[i];
> @@ -766,7 +766,7 @@ int avc_has_perm_noaudit(security_id_t ssid,
> avd_init(avd);
>
> if (!avc_using_threads && !avc_app_main_loop) {
> - (void)avc_netlink_check_nb();
> + (void) selinux_status_updated();
> }
>
> if (!aeref) {
> diff --git a/libselinux/src/avc_internal.c b/libselinux/src/avc_internal.c
> index 568a3d92..4ef92452 100644
> --- a/libselinux/src/avc_internal.c
> +++ b/libselinux/src/avc_internal.c
> @@ -53,6 +53,49 @@ int avc_enforcing = 1;
> int avc_setenforce = 0;
> int avc_netlink_trouble = 0;
>
> +/* process setenforce events for netlink and sestatus */
> +int avc_process_setenforce(int enforcing)
> +{
> + int rc = 0;
> +
> + avc_log(SELINUX_INFO,
> + "%s: received setenforce notice (enforcing=%d)\n",
> + avc_prefix, enforcing);
> + if (avc_setenforce)
> + goto out;
> + avc_enforcing = enforcing;
> + if (avc_enforcing && (rc = avc_ss_reset(0)) < 0) {
> + avc_log(SELINUX_ERROR,
> + "%s: cache reset returned %d (errno %d)\n",
> + avc_prefix, rc, errno);
> + return rc;
> + }
> +
> +out:
> + return selinux_netlink_setenforce(enforcing);
> +}
> +
> +/* process policyload events for netlink and sestatus */
> +int avc_process_policyload(uint32_t seqno)
> +{
> + int rc = 0;
> +
> + avc_log(SELINUX_INFO,
> + "%s: received policyload notice (seqno=%u)\n",
> + avc_prefix, seqno);
> + rc = avc_ss_reset(seqno);
> + if (rc < 0) {
> + avc_log(SELINUX_ERROR,
> + "%s: cache reset returned %d (errno %d)\n",
> + avc_prefix, rc, errno);
> + return rc;
> + }
> +
> + selinux_flush_class_cache();
> +
> + return selinux_netlink_policyload(seqno);
> +}
> +
> /* netlink socket code */
> static int fd = -1;
>
> @@ -177,20 +220,7 @@ static int avc_netlink_process(void *buf)
>
> case SELNL_MSG_SETENFORCE:{
> struct selnl_msg_setenforce *msg = NLMSG_DATA(nlh);
> - msg->val = !!msg->val;
> - avc_log(SELINUX_INFO,
> - "%s: received setenforce notice (enforcing=%d)\n",
> - avc_prefix, msg->val);
> - if (avc_setenforce)
> - break;
> - avc_enforcing = msg->val;
> - if (avc_enforcing && (rc = avc_ss_reset(0)) < 0) {
> - avc_log(SELINUX_ERROR,
> - "%s: cache reset returned %d (errno %d)\n",
> - avc_prefix, rc, errno);
> - return rc;
> - }
> - rc = selinux_netlink_setenforce(msg->val);
> + rc = avc_process_setenforce(!!msg->val);
> if (rc < 0)
> return rc;
> break;
> @@ -198,18 +228,7 @@ static int avc_netlink_process(void *buf)
>
> case SELNL_MSG_POLICYLOAD:{
> struct selnl_msg_policyload *msg = NLMSG_DATA(nlh);
> - avc_log(SELINUX_INFO,
> - "%s: received policyload notice (seqno=%u)\n",
> - avc_prefix, msg->seqno);
> - rc = avc_ss_reset(msg->seqno);
> - if (rc < 0) {
> - avc_log(SELINUX_ERROR,
> - "%s: cache reset returned %d (errno %d)\n",
> - avc_prefix, rc, errno);
> - return rc;
> - }
> - selinux_flush_class_cache();
> - rc = selinux_netlink_policyload(msg->seqno);
> + rc = avc_process_policyload(msg->seqno);
> if (rc < 0)
> return rc;
> break;
> @@ -284,6 +303,17 @@ void avc_netlink_loop(void)
>
> int avc_netlink_acquire_fd(void)
> {
> + if (fd < 0) {
> + int rc = 0;
> + rc = avc_netlink_open(0);
> + if (rc < 0) {
> + avc_log(SELINUX_ERROR,
> + "%s: could not open netlink socket: %d (%s)\n",
> + avc_prefix, errno, strerror(errno));
> + return rc;
> + }
> + }
> +
> avc_app_main_loop = 1;
>
> return fd;
> diff --git a/libselinux/src/avc_internal.h b/libselinux/src/avc_internal.h
> index 3f8a6bb1..2dda6490 100644
> --- a/libselinux/src/avc_internal.h
> +++ b/libselinux/src/avc_internal.h
> @@ -32,6 +32,13 @@ extern void (*avc_func_get_lock) (void *);
> extern void (*avc_func_release_lock) (void *);
> extern void (*avc_func_free_lock) (void *);
>
> +/* selinux status processing for netlink and sestatus */
> +extern int avc_process_setenforce(int enforcing);
> +extern int avc_process_policyload(uint32_t seqno);
> +
> +/* watch selinux status page */
> +extern void selinux_status_loop(void);
> +
> static inline void set_callbacks(const struct avc_memory_callback *mem_cb,
> const struct avc_log_callback *log_cb,
> const struct avc_thread_callback *thread_cb,
> diff --git a/libselinux/src/checkAccess.c b/libselinux/src/checkAccess.c
> index 3491fded..b337ea64 100644
> --- a/libselinux/src/checkAccess.c
> +++ b/libselinux/src/checkAccess.c
> @@ -39,7 +39,7 @@ int selinux_check_access(const char *scon, const char *tcon, const char *class,
> if (rc < 0)
> return rc;
>
> - (void) avc_netlink_check_nb();
> + (void) selinux_status_updated();
>
> sclass = string_to_security_class(class);
> if (sclass == 0) {
> diff --git a/libselinux/src/libselinux.map b/libselinux/src/libselinux.map
> index 2a368e93..8d8d8fd2 100644
> --- a/libselinux/src/libselinux.map
> +++ b/libselinux/src/libselinux.map
> @@ -203,6 +203,7 @@ LIBSELINUX_1.0 {
> selinux_status_close;
> selinux_status_deny_unknown;
> selinux_status_getenforce;
> + selinux_status_loop;
Looks like this snuck back in somehow. I'll get rid of it in a v4.
> selinux_status_open;
> selinux_status_policyload;
> selinux_status_updated;
> diff --git a/libselinux/src/sestatus.c b/libselinux/src/sestatus.c
> index 86267ff8..cbc7dbbe 100644
> --- a/libselinux/src/sestatus.c
> +++ b/libselinux/src/sestatus.c
> @@ -39,6 +39,7 @@ struct selinux_status_t
> static struct selinux_status_t *selinux_status = NULL;
> static int selinux_status_fd;
> static uint32_t last_seqno;
> +static uint32_t last_policyload;
>
> static uint32_t fallback_sequence;
> static int fallback_enforcing;
> @@ -116,6 +117,15 @@ int selinux_status_updated(void)
>
> if (last_seqno != curr_seqno)
> {
> + if (avc_enforcing != (int) selinux_status->enforcing) {
> + if (avc_process_setenforce(selinux_status->enforcing) < 0)
> + return -1;
> + }
> + if (last_policyload != selinux_status->policyload) {
> + if (avc_process_policyload(selinux_status->policyload) < 0)
> + return -1;
> + last_policyload = selinux_status->policyload;
> + }
> last_seqno = curr_seqno;
> result = 1;
> }
> @@ -316,6 +326,23 @@ error:
> return -1;
> }
>
> +/*
> + * selinux_status_loop
> + *
> + * Run routine for checking kernel status page in a listening thread.
> + * Falls back on netlink socket in the event of failure to open status page.
> + */
> +void selinux_status_loop(void)
> +{
> + /* Check kernel status page until error occurs */
> + while (selinux_status_updated() >= 0)
> + ;
> +
> + avc_log(SELINUX_ERROR,
> + "%s: status thread terminating due to error: %d (%s)\n",
> + avc_prefix, errno, strerror(errno));
> +}
> +
> /*
> * selinux_status_close
> *
> --
> 2.27.0
>
--
Mike Palmiotto
https://crunchydata.com
next prev parent reply other threads:[~2020-07-23 22:41 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-07-23 22:38 [PATCH v3] libselinux: use kernel status page by default Mike Palmiotto
2020-07-23 22:41 ` Mike Palmiotto [this message]
2020-07-24 12:18 ` Stephen Smalley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAMN686H-0KVjM7KpMuwMP9J1vOd00oZ=9agm_nEvUUP8fMfVJg@mail.gmail.com' \
--to=mike.palmiotto@crunchydata.com \
--cc=selinux@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).