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