lsm: consolidate buffer size handling into lsm_fill_user_ctx()

While we have a lsm_fill_user_ctx() helper function designed to make
life easier for LSMs which return lsm_ctx structs to userspace, we
didn't include all of the buffer length safety checks and buffer
padding adjustments in the helper.  This led to code duplication
across the different LSMs and the possibility for mistakes across the
different LSM subsystems.  In order to reduce code duplication and
decrease the chances of silly mistakes, we're consolidating all of
this code into the lsm_fill_user_ctx() helper.

The buffer padding is also modified from a fixed 8-byte alignment to
an alignment that matches the word length of the machine
(BITS_PER_LONG / 8).

Signed-off-by: Paul Moore <paul@paul-moore.com>

diff --git a/include/linux/security.h b/include/linux/security.h
index 334f75aa728991b1405a2f4daa13b8d7d27d176b..750130a7b9dd2ffb8248914b53c6a2a5d0774cd1 100644 (file)
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -492,8 +492,8 @@ int security_inode_notifysecctx(struct inode *inode, void *ctx, u32 ctxlen);
 int security_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen);
 int security_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen);
 int security_locked_down(enum lockdown_reason what);
-int lsm_fill_user_ctx(struct lsm_ctx __user *ctx, void *context,
-                     size_t context_size, u64 id, u64 flags);
+int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, size_t *uctx_len,
+                     void *val, size_t val_len, u64 id, u64 flags);
 #else /* CONFIG_SECURITY */
 
 static inline int call_blocking_lsm_notifier(enum lsm_event event, void *data)
@@ -1424,8 +1424,9 @@ static inline int security_locked_down(enum lockdown_reason what)
 {
        return 0;
 }
-static inline int lsm_fill_user_ctx(struct lsm_ctx __user *ctx, void *context,
-                                   size_t context_size, u64 id, u64 flags)
+static inline int lsm_fill_user_ctx(struct lsm_ctx __user *uctx,
+                                   size_t *uctx_len, void *val, size_t val_len,
+                                   u64 id, u64 flags)
 {
        return -EOPNOTSUPP;
 }

diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index 8165f80c10ffcc4f9939ac1df6d8ad897b23f70c..332198e0a017ce62b2cd259612ed67f71bde1788 100644 (file)
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -782,7 +782,6 @@ static int apparmor_getselfattr(unsigned int attr, struct lsm_ctx __user *lx,
        int error = -ENOENT;
        struct aa_task_ctx *ctx = task_ctx(current);
        struct aa_label *label = NULL;
-       size_t total_len = 0;
        char *value;
 
        switch (attr) {
@@ -804,22 +803,14 @@ static int apparmor_getselfattr(unsigned int attr, struct lsm_ctx __user *lx,
 
        if (label) {
                error = aa_getprocattr(label, &value, false);
-               if (error > 0) {
-                       total_len = ALIGN(struct_size(lx, ctx, error), 8);
-                       if (total_len > *size)
-                               error = -E2BIG;
-                       else if (lx)
-                               error = lsm_fill_user_ctx(lx, value, error,
-                                                         LSM_ID_APPARMOR, 0);
-                       else
-                               error = 1;
-               }
+               if (error > 0)
+                       error = lsm_fill_user_ctx(lx, size, value, error,
+                                                 LSM_ID_APPARMOR, 0);
                kfree(value);
        }
 
        aa_put_label(label);
 
-       *size = total_len;
        if (error < 0)
                return error;
        return 1;

diff --git a/security/security.c b/security/security.c
index 78e7ffcc9f6cd3aa63e8056e531efd60fc4f9fed..86f7a19959910d4d455f8eb7744ae2467e015150 100644 (file)
--- a/security/security.c
+++ b/security/security.c
@@ -772,42 +772,49 @@ static int lsm_superblock_alloc(struct super_block *sb)
 
 /**
  * lsm_fill_user_ctx - Fill a user space lsm_ctx structure
- * @ctx: an LSM context to be filled
- * @context: the new context value
- * @context_size: the size of the new context value
+ * @uctx: a userspace LSM context to be filled
+ * @uctx_len: available uctx size (input), used uctx size (output)
+ * @val: the new LSM context value
+ * @val_len: the size of the new LSM context value
  * @id: LSM id
  * @flags: LSM defined flags
  *
- * Fill all of the fields in a user space lsm_ctx structure.
- * Caller is assumed to have verified that @ctx has enough space
- * for @context.
+ * Fill all of the fields in a userspace lsm_ctx structure.
  *
- * Returns 0 on success, -EFAULT on a copyout error, -ENOMEM
- * if memory can't be allocated.
+ * Returns 0 on success, -E2BIG if userspace buffer is not large enough,
+ * -EFAULT on a copyout error, -ENOMEM if memory can't be allocated.
  */
-int lsm_fill_user_ctx(struct lsm_ctx __user *ctx, void *context,
-                     size_t context_size, u64 id, u64 flags)
+int lsm_fill_user_ctx(struct lsm_ctx __user *uctx, size_t *uctx_len,
+                     void *val, size_t val_len,
+                     u64 id, u64 flags)
 {
-       struct lsm_ctx *lctx;
-       size_t locallen = struct_size(lctx, ctx, context_size);
+       struct lsm_ctx *nctx = NULL;
+       size_t nctx_len;
        int rc = 0;
 
-       lctx = kzalloc(locallen, GFP_KERNEL);
-       if (lctx == NULL)
-               return -ENOMEM;
-
-       lctx->id = id;
-       lctx->flags = flags;
-       lctx->ctx_len = context_size;
-       lctx->len = locallen;
+       nctx_len = ALIGN(struct_size(nctx, ctx, val_len), BITS_PER_LONG / 8);
+       if (nctx_len > *uctx_len) {
+               rc = -E2BIG;
+               goto out;
+       }
 
-       memcpy(lctx->ctx, context, context_size);
+       nctx = kzalloc(nctx_len, GFP_KERNEL);
+       if (nctx == NULL) {
+               rc = -ENOMEM;
+               goto out;
+       }
+       nctx->id = id;
+       nctx->flags = flags;
+       nctx->len = nctx_len;
+       nctx->ctx_len = val_len;
+       memcpy(nctx->ctx, val, val_len);
 
-       if (copy_to_user(ctx, lctx, locallen))
+       if (copy_to_user(uctx, nctx, nctx_len))
                rc = -EFAULT;
 
-       kfree(lctx);
-
+out:
+       kfree(nctx);
+       *uctx_len = nctx_len;
        return rc;
 }
 

diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index b6c7930a3ab29cb01df39fb2d9919d82247b050d..942f2b8c4ebb2f0136491f31d63c81138ad4fba5 100644 (file)
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6486,30 +6486,32 @@ abort_change:
        return error;
 }
 
+/**
+ * selinux_getselfattr - Get SELinux current task attributes
+ * @attr: the requested attribute
+ * @ctx: buffer to receive the result
+ * @size: buffer size (input), buffer size used (output)
+ * @flags: unused
+ *
+ * Fill the passed user space @ctx with the details of the requested
+ * attribute.
+ *
+ * Returns the number of attributes on success, an error code otherwise.
+ * There will only ever be one attribute.
+ */
 static int selinux_getselfattr(unsigned int attr, struct lsm_ctx __user *ctx,
                               size_t *size, u32 flags)
 {
-       char *value;
-       size_t total_len;
-       int len;
-       int rc = 0;
-
-       len = selinux_lsm_getattr(attr, current, &value);
-       if (len < 0)
-               return len;
-
-       total_len = ALIGN(struct_size(ctx, ctx, len), 8);
-
-       if (total_len > *size)
-               rc = -E2BIG;
-       else if (ctx)
-               rc = lsm_fill_user_ctx(ctx, value, len, LSM_ID_SELINUX, 0);
-
-       kfree(value);
-       *size = total_len;
-       if (rc < 0)
-               return rc;
-       return 1;
+       int rc;
+       char *val;
+       int val_len;
+
+       val_len = selinux_lsm_getattr(attr, current, &val);
+       if (val_len < 0)
+               return val_len;
+       rc = lsm_fill_user_ctx(ctx, size, val, val_len, LSM_ID_SELINUX, 0);
+       kfree(val);
+       return (!rc ? 1 : rc);
 }
 
 static int selinux_setselfattr(unsigned int attr, struct lsm_ctx *ctx,

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 12160d060cc133b1ede3f08a5f73bb76975e15d5..99664c8cf86759cfd856a2ba566ab77f1dbba5b0 100644 (file)
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3642,28 +3642,17 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
 static int smack_getselfattr(unsigned int attr, struct lsm_ctx __user *ctx,
                             size_t *size, u32 flags)
 {
-       struct smack_known *skp = smk_of_current();
-       int total;
-       int slen;
        int rc;
+       struct smack_known *skp;
 
        if (attr != LSM_ATTR_CURRENT)
                return -EOPNOTSUPP;
 
-       slen = strlen(skp->smk_known) + 1;
-       total = ALIGN(slen + sizeof(*ctx), 8);
-       if (total > *size)
-               rc = -E2BIG;
-       else if (ctx)
-               rc = lsm_fill_user_ctx(ctx, skp->smk_known, slen, LSM_ID_SMACK,
-                                      0);
-       else
-               rc = 1;
-
-       *size = total;
-       if (rc >= 0)
-               return 1;
-       return rc;
+       skp = smk_of_current();
+       rc = lsm_fill_user_ctx(ctx, size,
+                              skp->smk_known, strlen(skp->smk_known) + 1,
+                              LSM_ID_SMACK, 0);
+       return (!rc ? 1 : rc);
 }
 
 /**