On 7/12/2024 9:31 PM, Simon Horman wrote:
On Thu, Jul 11, 2024 at 07:18:51PM +0800, Xu Kuohai wrote:
From: Xu Kuohai <xukuohai@xxxxxxxxxx>
To be consistent with most LSM hooks, convert the return value of
hook inode_getsecurity to 0 or a negative error code.
Before:
- Hook inode_getsecurity returns size of buffer on success or a
negative error code on failure.
After:
- Hook inode_getsecurity returns 0 on success or a negative error
code on failure. An output parameter @len is introduced to hold
the buffer size on success.
Signed-off-by: Xu Kuohai <xukuohai@xxxxxxxxxx>
---
fs/xattr.c | 19 ++++++++++---------
include/linux/lsm_hook_defs.h | 3 ++-
include/linux/security.h | 12 ++++++------
security/commoncap.c | 9 ++++++---
security/security.c | 11 ++++++-----
security/selinux/hooks.c | 16 ++++++----------
security/smack/smack_lsm.c | 14 +++++++-------
7 files changed, 43 insertions(+), 41 deletions(-)
diff --git a/fs/xattr.c b/fs/xattr.c
index f8b643f91a98..f4e3bedf7272 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -339,27 +339,28 @@ xattr_getsecurity(struct mnt_idmap *idmap, struct inode *inode,
const char *name, void *value, size_t size)
{
void *buffer = NULL;
- ssize_t len;
+ int error;
+ u32 len;
if (!value || !size) {
- len = security_inode_getsecurity(idmap, inode, name,
- &buffer, false);
+ error = security_inode_getsecurity(idmap, inode, name,
+ false, &buffer, &len);
goto out_noalloc;
}
- len = security_inode_getsecurity(idmap, inode, name, &buffer,
- true);
- if (len < 0)
- return len;
+ error = security_inode_getsecurity(idmap, inode, name, true,
+ &buffer, &len);
+ if (error)
+ return error;
if (size < len) {
- len = -ERANGE;
+ error = -ERANGE;
goto out;
}
memcpy(value, buffer, len);
out:
kfree(buffer);
out_noalloc:
- return len;
+ return error < 0 ? error : len;
Hi Xu Kuohai,
len is an unsigned 32-bit entity, but the return type of this function
is an unsigned value (ssize_t). So in theory, if len is very large,
a negative error value error will be returned.
}
Similarly for the handling of nattr in lsm_get_self_attr in
lsm_syscalls.c in a subsequent patch.
Flagged by Smatch.
...
OK, indeed there is no standardization that says ssize_t must be wider than u32.
I'll look into adding overflow checks. Thanks.
