From: Xu Kuohai <xukuo...@huawei.com>

To be consistent with most LSM hooks, convert the return value of
hook key_getsecurity to 0 or a negative error code.

Before:
- Hook key_getsecurity returns length of value on success or a
  negative error code on failure.

After:
- Hook key_getsecurity returns 0 on success or a negative error
  code on failure. An output parameter @len is introduced to hold
  the length of value on success.

Signed-off-by: Xu Kuohai <xukuo...@huawei.com>
---
 include/linux/lsm_hook_defs.h |  3 ++-
 include/linux/security.h      |  6 ++++--
 security/keys/keyctl.c        | 11 ++++++++---
 security/security.c           | 26 +++++++++++++++++++++-----
 security/selinux/hooks.c      | 11 +++++------
 security/smack/smack_lsm.c    | 21 +++++++++++----------
 6 files changed, 51 insertions(+), 27 deletions(-)

diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index b0e3cf3fc33f..54fec360947c 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -407,7 +407,8 @@ LSM_HOOK(int, 0, key_alloc, struct key *key, const struct 
cred *cred,
 LSM_HOOK(void, LSM_RET_VOID, key_free, struct key *key)
 LSM_HOOK(int, 0, key_permission, key_ref_t key_ref, const struct cred *cred,
         enum key_need_perm need_perm)
-LSM_HOOK(int, 0, key_getsecurity, struct key *key, char **buffer)
+LSM_HOOK(int, 0, key_getsecurity, struct key *key, char **buffer,
+        size_t *len)
 LSM_HOOK(void, LSM_RET_VOID, key_post_create_or_update, struct key *keyring,
         struct key *key, const void *payload, size_t payload_len,
         unsigned long flags, bool create)
diff --git a/include/linux/security.h b/include/linux/security.h
index 616047030a89..4e64e51a1a86 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -2020,7 +2020,7 @@ int security_key_alloc(struct key *key, const struct cred 
*cred, unsigned long f
 void security_key_free(struct key *key);
 int security_key_permission(key_ref_t key_ref, const struct cred *cred,
                            enum key_need_perm need_perm);
-int security_key_getsecurity(struct key *key, char **_buffer);
+int security_key_getsecurity(struct key *key, char **_buffer, size_t *_len);
 void security_key_post_create_or_update(struct key *keyring, struct key *key,
                                        const void *payload, size_t payload_len,
                                        unsigned long flags, bool create);
@@ -2045,9 +2045,11 @@ static inline int security_key_permission(key_ref_t 
key_ref,
        return 0;
 }
 
-static inline int security_key_getsecurity(struct key *key, char **_buffer)
+static inline int security_key_getsecurity(struct key *key, char **_buffer,
+                                          size_t *_len)
 {
        *_buffer = NULL;
+       *_len = 0;
        return 0;
 }
 
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index 4bc3e9398ee3..e9f857620f28 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1565,6 +1565,7 @@ long keyctl_get_security(key_serial_t keyid,
        struct key *key, *instkey;
        key_ref_t key_ref;
        char *context;
+       size_t len;
        long ret;
 
        key_ref = lookup_user_key(keyid, KEY_LOOKUP_PARTIAL, KEY_NEED_VIEW);
@@ -1586,15 +1587,18 @@ long keyctl_get_security(key_serial_t keyid,
        }
 
        key = key_ref_to_ptr(key_ref);
-       ret = security_key_getsecurity(key, &context);
-       if (ret == 0) {
+       ret = security_key_getsecurity(key, &context, &len);
+       if (ret < 0)
+               goto error;
+       if (len == 0) {
                /* if no information was returned, give userspace an empty
                 * string */
                ret = 1;
                if (buffer && buflen > 0 &&
                    copy_to_user(buffer, "", 1) != 0)
                        ret = -EFAULT;
-       } else if (ret > 0) {
+       } else {
+               ret = len;
                /* return as much data as there's room for */
                if (buffer && buflen > 0) {
                        if (buflen > ret)
@@ -1607,6 +1611,7 @@ long keyctl_get_security(key_serial_t keyid,
                kfree(context);
        }
 
+error:
        key_ref_put(key_ref);
        return ret;
 }
diff --git a/security/security.c b/security/security.c
index 9dd2ae6cf763..2c161101074d 100644
--- a/security/security.c
+++ b/security/security.c
@@ -5338,19 +5338,35 @@ int security_key_permission(key_ref_t key_ref, const 
struct cred *cred,
  * security_key_getsecurity() - Get the key's security label
  * @key: key
  * @buffer: security label buffer
+ * @len: the length of @buffer (including terminating NULL) on success
  *
  * Get a textual representation of the security context attached to a key for
  * the purposes of honouring KEYCTL_GETSECURITY.  This function allocates the
  * storage for the NUL-terminated string and the caller should free it.
  *
- * Return: Returns the length of @buffer (including terminating NUL) or -ve if
- *         an error occurs.  May also return 0 (and a NULL buffer pointer) if
- *         there is no security label assigned to the key.
+ * Return: Returns 0 on success or -ve if an error occurs. May also return 0
+ *         (and a NULL buffer pointer) if there is no security label assigned
+ *         to the key.
  */
-int security_key_getsecurity(struct key *key, char **buffer)
+int security_key_getsecurity(struct key *key, char **buffer, size_t *len)
 {
+       int rc;
+       size_t n = 0;
+       struct security_hook_list *hp;
+
        *buffer = NULL;
-       return call_int_hook(key_getsecurity, key, buffer);
+
+       hlist_for_each_entry(hp, &security_hook_heads.key_getsecurity, list) {
+               rc = hp->hook.key_getsecurity(key, buffer, &n);
+               if (rc < 0)
+                       return rc;
+               if (n)
+                       break;
+       }
+
+       *len = n;
+
+       return 0;
 }
 
 /**
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 16cd336aab3d..747ec602dec0 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6737,18 +6737,17 @@ static int selinux_key_permission(key_ref_t key_ref,
        return avc_has_perm(sid, ksec->sid, SECCLASS_KEY, perm, NULL);
 }
 
-static int selinux_key_getsecurity(struct key *key, char **_buffer)
+static int selinux_key_getsecurity(struct key *key, char **_buffer,
+                                  size_t *_len)
 {
        struct key_security_struct *ksec = key->security;
        char *context = NULL;
-       unsigned len;
+       u32 context_len;
        int rc;
 
-       rc = security_sid_to_context(ksec->sid,
-                                    &context, &len);
-       if (!rc)
-               rc = len;
+       rc = security_sid_to_context(ksec->sid, &context, &context_len);
        *_buffer = context;
+       *_len = context_len;
        return rc;
 }
 
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 8a352bd05565..9a121ad53b16 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -4585,30 +4585,31 @@ static int smack_key_permission(key_ref_t key_ref,
 /*
  * smack_key_getsecurity - Smack label tagging the key
  * @key points to the key to be queried
- * @_buffer points to a pointer that should be set to point to the
- * resulting string (if no label or an error occurs).
- * Return the length of the string (including terminating NUL) or -ve if
- * an error.
- * May also return 0 (and a NULL buffer pointer) if there is no label.
+ * @_buffer points to a pointer that should be set to point to the resulting
+ *          string (if no label or an error occurs).
+ * @_len  the length of the @_buffer (including terminating NUL)
+ *
+ * Return 0 on success or -ve if an error.
+ * If there is no label, @_buffer will be set to NULL and @_len will be set to
+ * 0.
  */
-static int smack_key_getsecurity(struct key *key, char **_buffer)
+static int smack_key_getsecurity(struct key *key, char **_buffer, size_t *_len)
 {
        struct smack_known *skp = key->security;
-       size_t length;
        char *copy;
 
        if (key->security == NULL) {
                *_buffer = NULL;
+               *_len = 0;
                return 0;
        }
 
        copy = kstrdup(skp->smk_known, GFP_KERNEL);
        if (copy == NULL)
                return -ENOMEM;
-       length = strlen(copy) + 1;
-
+       *_len = strlen(copy) + 1;
        *_buffer = copy;
-       return length;
+       return 0;
 }
 
 
-- 
2.30.2


Reply via email to