Re: [PATCH v2] selinux: support wildcard match in genfscon

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

 



Mar 11, 2025 10:29:42 Takaya Saeki <takayas@xxxxxxxxxxxx>:

> Currently, genfscon only supports string prefix match to label files.
> Thus, labeling numerous dynamic sysfs entries requires many specific
> path rules. For example, labeling device paths such as
> `/sys/devices/pci0000:00/0000:00:03.1/<...>/0000:04:00.1/wakeup`
> requires listing all specific PCI paths, which is challenging to
> maintain. While user-space restorecon can handle these paths with
> regular expression rules, but relabeling thousands of paths under sysfs
> after it is mounted is inefficient compared to using genfscon.
>
> This commit adds wildcard match to support rules efficient but
> expressive enough. This allows users to create fine-grained sysfs rules
> without burden of listing specific paths. When multiple wildcard rules
> match against a path, then the longest rule (determined by the length of
> the rule string) will be applied. If multiple rules of the same length
> match, the first matching rule encountered in the genfscon policy will
> be applied. However, users are encouraged to write longer, more explicit
> path rules to avoid relying on this behavior.
>
> This change resulted in nice real-world performance improvements. For
> example, boot times on test Android devices were reduced by 15%. This
> improvement is due to the elimination of the "restorecon -R /sys" step
> during boot, which takes more than two seconds in the worst case.
>
> Signed-off-by: Takaya Saeki <takayas@xxxxxxxxxxxx>
> ---
> Changelog between v2 and v1
> - Use given genfs rules by the userspace as is, instead of appending "*".
> - Fix __security_genfs_sid hadn't checked caps of the given argument.
> - Fix the wrong strncmp usage bug.
>
> security/selinux/include/policycap.h       |  1 +
> security/selinux/include/policycap_names.h |  1 +
> security/selinux/ss/services.c             | 20 ++++++++++++++++----
> 3 files changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/security/selinux/include/policycap.h b/security/selinux/include/policycap.h
> index 079679fe7254..2b4014a826f0 100644
> --- a/security/selinux/include/policycap.h
> +++ b/security/selinux/include/policycap.h
> @@ -15,6 +15,7 @@ enum {
>     POLICYDB_CAP_IOCTL_SKIP_CLOEXEC,
>     POLICYDB_CAP_USERSPACE_INITIAL_CONTEXT,
>     POLICYDB_CAP_NETLINK_XPERM,
> +   POLICYDB_CAP_GENFS_SECLABEL_WILDCARD,
>     __POLICYDB_CAP_MAX
> };
> #define POLICYDB_CAP_MAX (__POLICYDB_CAP_MAX - 1)
> diff --git a/security/selinux/include/policycap_names.h b/security/selinux/include/policycap_names.h
> index e080827408c4..1053f2c95ff3 100644
> --- a/security/selinux/include/policycap_names.h
> +++ b/security/selinux/include/policycap_names.h
> @@ -18,6 +18,7 @@ const char *const selinux_policycap_names[__POLICYDB_CAP_MAX] = {
>     "ioctl_skip_cloexec",
>     "userspace_initial_context",
>     "netlink_xperm",
> +   "genfs_seclabel_wildcard",
> };
> /* clang-format on */
>
> diff --git a/security/selinux/ss/services.c b/security/selinux/ss/services.c
> index 8478842fbf9e..9f98c9dc71f6 100644
> --- a/security/selinux/ss/services.c
> +++ b/security/selinux/ss/services.c
> @@ -48,6 +48,7 @@
> #include <linux/audit.h>
> #include <linux/vmalloc.h>
> #include <linux/lsm_hooks.h>
> +#include <linux/parser.h>
> #include <net/netlabel.h>
>
> #include "flask.h"
> @@ -2863,6 +2864,7 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
>     struct genfs *genfs;
>     struct ocontext *c;
>     int cmp = 0;
> +   bool wildcard = 0;
>
>     while (path[0] == '/' && path[1] == '/')
>         path++;
> @@ -2879,11 +2881,21 @@ static inline int __security_genfs_sid(struct selinux_policy *policy,
>     if (!genfs || cmp)
>         return -ENOENT;
>
> +
> +   wildcard = ebitmap_get_bit(&policy->policydb.policycaps,
> +                  POLICYDB_CAP_GENFS_SECLABEL_WILDCARD);

Commonly policy capabilities are gathered via a helper from security.h from selinux_state, shouldn't it here too?

>     for (c = genfs->head; c; c = c->next) {
> -       size_t len = strlen(c->u.name);
> -       if ((!c->v.sclass || sclass == c->v.sclass) &&
> -           (strncmp(c->u.name, path, len) == 0))
> -           break;
> +       if (!c->v.sclass || sclass == c->v.sclass) {
> +           if (wildcard) {
> +               if (match_wildcard(c->u.name, path))
> +                   break;
> +           } else {
> +               size_t len = strlen(c->u.name);
> +
> +               if ((strncmp(c->u.name, path, len)) == 0)
> +                   break;
> +           }
> +       }
>     }
>
>     if (!c)
> --
> 2.49.0.rc1.451.g8f38331e32-goog






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

  Powered by Linux