[PATCH bpf-next v4 09/20] lsm: Refactor return value of LSM hook key_getsecurity

Xu Kuohai
2024-07-11T11:14:08Z

From: Xu Kuohai <xukuohai@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 <xukuohai@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