On Sat, Jul 20, 2024 at 5:31 AM Xu Kuohai xukuohai@huaweicloud.com wrote:
On 7/19/2024 10:08 AM, Paul Moore wrote:
On Jul 11, 2024 Xu Kuohai xukuohai@huaweicloud.com wrote:
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/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; }
Help me understand why we can't continue to use the call_int_hook() macro here?
Before this patch, the hook may return +ve, 0, or -ve, and call_int_hook breaks the loop when the hook return value is not 0.
After this patch, the +ve is stored in @n, so @n and return value should both be checked to determine whether to break the loop. This is not feasible with call_int_hook.
Yes, gotcha. I was focused on the error condition and wasn't thinking about the length getting zero'd out by a trailing callback. Unfortunately, we *really* want to stick with the call_{int,void}_hook() macros so I think we either need to find a way to work within that constraint for existing macro callers, or we have to leave this hook as-is for the moment.