cachefiles: Parse the "secctx" immediately

Instead of storing an opaque string, call security_secctx_to_secid()
right in the "secctx" command handler and store only the numeric
"secid".  This eliminates an unnecessary string allocation and allows
the daemon to receive errors when writing the "secctx" command instead
of postponing the error to the "bind" command handler.  For example,
if the kernel was built without `CONFIG_SECURITY`, "bind" will return
`EOPNOTSUPP`, but the daemon doesn't know why.  With this patch, the
"secctx" will instead return `EOPNOTSUPP` which is the right context
for this error.

This patch adds a boolean flag `have_secid` because I'm not sure if we
can safely assume that zero is the special secid value for "not set".
This appears to be true for SELinux, Smack and AppArmor, but since
this attribute is not documented, I'm unable to derive a stable
guarantee for that.

Signed-off-by: Max Kellermann <max.kellermann@ionos.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Link: https://lore.kernel.org/r/20241209141554.638708-1-max.kellermann@ionos.com/
Link: https://lore.kernel.org/r/20241213135013.2964079-6-dhowells@redhat.com
Signed-off-by: Christian Brauner <brauner@kernel.org>

diff --git a/fs/cachefiles/daemon.c b/fs/cachefiles/daemon.c
index 89b11336a83697298225e0dc61a0dfa6a43910cd..1806bff8e59bc348a9645a8820de1c46c193d0b7 100644 (file)
--- a/fs/cachefiles/daemon.c
+++ b/fs/cachefiles/daemon.c
@@ -15,6 +15,7 @@
 #include <linux/namei.h>
 #include <linux/poll.h>
 #include <linux/mount.h>
+#include <linux/security.h>
 #include <linux/statfs.h>
 #include <linux/ctype.h>
 #include <linux/string.h>
@@ -576,7 +577,7 @@ static int cachefiles_daemon_dir(struct cachefiles_cache *cache, char *args)
  */
 static int cachefiles_daemon_secctx(struct cachefiles_cache *cache, char *args)
 {
-       char *secctx;
+       int err;
 
        _enter(",%s", args);
 
@@ -585,16 +586,16 @@ static int cachefiles_daemon_secctx(struct cachefiles_cache *cache, char *args)
                return -EINVAL;
        }
 
-       if (cache->secctx) {
+       if (cache->have_secid) {
                pr_err("Second security context specified\n");
                return -EINVAL;
        }
 
-       secctx = kstrdup(args, GFP_KERNEL);
-       if (!secctx)
-               return -ENOMEM;
+       err = security_secctx_to_secid(args, strlen(args), &cache->secid);
+       if (err)
+               return err;
 
-       cache->secctx = secctx;
+       cache->have_secid = true;
        return 0;
 }
 
@@ -820,7 +821,6 @@ static void cachefiles_daemon_unbind(struct cachefiles_cache *cache)
        put_cred(cache->cache_cred);
 
        kfree(cache->rootdirname);
-       kfree(cache->secctx);
        kfree(cache->tag);
 
        _leave("");

diff --git a/fs/cachefiles/internal.h b/fs/cachefiles/internal.h
index 7b99bd98de75b8d95e09da1ca7cd1bb3378fcc62..38c236e38cef85cbf15696351475ea576235d2af 100644 (file)
--- a/fs/cachefiles/internal.h
+++ b/fs/cachefiles/internal.h
@@ -122,7 +122,6 @@ struct cachefiles_cache {
 #define CACHEFILES_STATE_CHANGED       3       /* T if state changed (poll trigger) */
 #define CACHEFILES_ONDEMAND_MODE       4       /* T if in on-demand read mode */
        char                            *rootdirname;   /* name of cache root directory */
-       char                            *secctx;        /* LSM security context */
        char                            *tag;           /* cache binding tag */
        refcount_t                      unbind_pincount;/* refcount to do daemon unbind */
        struct xarray                   reqs;           /* xarray of pending on-demand requests */
@@ -130,6 +129,8 @@ struct cachefiles_cache {
        struct xarray                   ondemand_ids;   /* xarray for ondemand_id allocation */
        u32                             ondemand_id_next;
        u32                             msg_id_next;
+       u32                             secid;          /* LSM security id */
+       bool                            have_secid;     /* whether "secid" was set */
 };
 
 static inline bool cachefiles_in_ondemand_mode(struct cachefiles_cache *cache)

diff --git a/fs/cachefiles/security.c b/fs/cachefiles/security.c
index fe777164f1d894578af6d7d3e136d6b21dce0e27..fc6611886b3b5e1107e5444623a2db0b65020b67 100644 (file)
--- a/fs/cachefiles/security.c
+++ b/fs/cachefiles/security.c
@@ -18,7 +18,7 @@ int cachefiles_get_security_ID(struct cachefiles_cache *cache)
        struct cred *new;
        int ret;
 
-       _enter("{%s}", cache->secctx);
+       _enter("{%u}", cache->have_secid ? cache->secid : 0);
 
        new = prepare_kernel_cred(current);
        if (!new) {
@@ -26,8 +26,8 @@ int cachefiles_get_security_ID(struct cachefiles_cache *cache)
                goto error;
        }
 
-       if (cache->secctx) {
-               ret = set_security_override_from_ctx(new, cache->secctx);
+       if (cache->have_secid) {
+               ret = set_security_override(new, cache->secid);
                if (ret < 0) {
                        put_cred(new);
                        pr_err("Security denies permission to nominate security context: error %d\n",