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

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

From: Xu Kuohai <xukuohai@huawei.com>

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@huawei.com>
---
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;
}

/*
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 964849de424b..4f056f2613af 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -169,7 +169,8 @@ LSM_HOOK(int, 0, inode_need_killpriv, struct dentry *dentry, bool *need)
LSM_HOOK(int, 0, inode_killpriv, struct mnt_idmap *idmap,
struct dentry *dentry)
LSM_HOOK(int, -EOPNOTSUPP, inode_getsecurity, struct mnt_idmap *idmap,
- struct inode *inode, const char *name, void **buffer, bool alloc)
+ struct inode *inode, const char *name, bool alloc, void **buffer,
+ u32 *len)
LSM_HOOK(int, -EOPNOTSUPP, inode_setsecurity, struct inode *inode,
const char *name, const void *value, size_t size, int flags)
LSM_HOOK(int, 0, inode_listsecurity, struct inode *inode, char *buffer,
diff --git a/include/linux/security.h b/include/linux/security.h
index 1614ef5b2dd2..b6d296d21438 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -164,8 +164,8 @@ int cap_inode_removexattr(struct mnt_idmap *idmap,
int cap_inode_need_killpriv(struct dentry *dentry, bool *need);
int cap_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry);
int cap_inode_getsecurity(struct mnt_idmap *idmap,
- struct inode *inode, const char *name, void **buffer,
- bool alloc);
+ struct inode *inode, const char *name, bool alloc,
+ void **buffer, u32 *len);
extern int cap_mmap_addr(unsigned long addr);
extern int cap_mmap_file(struct file *file, unsigned long reqprot,
unsigned long prot, unsigned long flags);
@@ -393,7 +393,7 @@ int security_inode_need_killpriv(struct dentry *dentry, int *attr);
int security_inode_killpriv(struct mnt_idmap *idmap, struct dentry *dentry);
int security_inode_getsecurity(struct mnt_idmap *idmap,
struct inode *inode, const char *name,
- void **buffer, bool alloc);
+ bool alloc, void **buffer, u32 *len);
int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags);
int security_inode_listsecurity(struct inode *inode, char *buffer, size_t buffer_size);
void security_inode_getsecid(struct inode *inode, u32 *secid);
@@ -996,10 +996,10 @@ static inline int security_inode_killpriv(struct mnt_idmap *idmap,

static inline int security_inode_getsecurity(struct mnt_idmap *idmap,
struct inode *inode,
- const char *name, void **buffer,
- bool alloc)
+ const char *name, bool alloc,
+ void **buffer, u32 *len)
{
- return cap_inode_getsecurity(idmap, inode, name, buffer, alloc);
+ return cap_inode_getsecurity(idmap, inode, name, alloc, buffer, len);
}

static inline int security_inode_setsecurity(struct inode *inode, const char *name, const void *value, size_t size, int flags)
diff --git a/security/commoncap.c b/security/commoncap.c
index 17d6188d22cf..ff82e2ab6f8f 100644
--- a/security/commoncap.c
+++ b/security/commoncap.c
@@ -383,8 +383,8 @@ static bool is_v3header(int size, const struct vfs_cap_data *cap)
* so that's good.
*/
int cap_inode_getsecurity(struct mnt_idmap *idmap,
- struct inode *inode, const char *name, void **buffer,
- bool alloc)
+ struct inode *inode, const char *name,
+ bool alloc, void **buffer, u32 *len)
{
int size;
kuid_t kroot;
@@ -485,7 +485,10 @@ int cap_inode_getsecurity(struct mnt_idmap *idmap,
}
out_free:
kfree(tmpbuf);
- return size;
+ if (size < 0)
+ return size;
+ *len = size;
+ return 0;
}

/**
diff --git a/security/security.c b/security/security.c
index a4abcd86eb36..614f14cbfff7 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2544,8 +2544,9 @@ int security_inode_killpriv(struct mnt_idmap *idmap,
* @idmap: idmap of the mount
* @inode: inode
* @name: xattr name
- * @buffer: security label buffer
* @alloc: allocation flag
+ * @buffer: security label buffer
+ * @len: security label length
*
* Retrieve a copy of the extended attribute representation of the security
* label associated with @name for @inode via @buffer. Note that @name is the
@@ -2553,17 +2554,17 @@ int security_inode_killpriv(struct mnt_idmap *idmap,
* @alloc is used to specify if the call should return a value via the buffer
* or just the value length.
*
- * Return: Returns size of buffer on success.
+ * Return: Returns 0 on success or a negative error code on failure.
*/
int security_inode_getsecurity(struct mnt_idmap *idmap,
struct inode *inode, const char *name,
- void **buffer, bool alloc)
+ bool alloc, void **buffer, u32 *len)
{
if (unlikely(IS_PRIVATE(inode)))
return LSM_RET_DEFAULT(inode_getsecurity);

- return call_int_hook(inode_getsecurity, idmap, inode, name, buffer,
- alloc);
+ return call_int_hook(inode_getsecurity, idmap, inode, name, alloc,
+ buffer, len);
}

/**
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 9cd5a8f1f6a3..70792bba24d9 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -3407,7 +3407,7 @@ static int selinux_path_notify(const struct path *path, u64 mask,
*/
static int selinux_inode_getsecurity(struct mnt_idmap *idmap,
struct inode *inode, const char *name,
- void **buffer, bool alloc)
+ bool alloc, void **buffer, u32 *len)
{
u32 size;
int error;
@@ -3440,14 +3440,14 @@ static int selinux_inode_getsecurity(struct mnt_idmap *idmap,
&context, &size);
if (error)
return error;
- error = size;
+ *len = size;
if (alloc) {
*buffer = context;
goto out_nofree;
}
kfree(context);
out_nofree:
- return error;
+ return 0;
}

static int selinux_inode_setsecurity(struct inode *inode, const char *name,
@@ -6644,13 +6644,9 @@ static int selinux_inode_setsecctx(struct dentry *dentry, void *ctx, u32 ctxlen)

static int selinux_inode_getsecctx(struct inode *inode, void **ctx, u32 *ctxlen)
{
- int len = 0;
- len = selinux_inode_getsecurity(&nop_mnt_idmap, inode,
- XATTR_SELINUX_SUFFIX, ctx, true);
- if (len < 0)
- return len;
- *ctxlen = len;
- return 0;
+ return selinux_inode_getsecurity(&nop_mnt_idmap, inode,
+ XATTR_SELINUX_SUFFIX,
+ true, ctx, ctxlen);
}
#ifdef CONFIG_KEYS

diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index f5cbec1e6a92..e7a5f6fd9a2d 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -1543,14 +1543,15 @@ static int smack_inode_remove_acl(struct mnt_idmap *idmap,
* @idmap: idmap of the mount
* @inode: the object
* @name: attribute name
- * @buffer: where to put the result
* @alloc: duplicate memory
+ * @buffer: where to put the result
+ * @len: where to put the result length
*
- * Returns the size of the attribute or an error code
+ * Returns 0 on success or a negative error code on failure
*/
static int smack_inode_getsecurity(struct mnt_idmap *idmap,
struct inode *inode, const char *name,
- void **buffer, bool alloc)
+ bool alloc, void **buffer, u32 *len)
{
struct socket_smack *ssp;
struct socket *sock;
@@ -1558,7 +1559,6 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
struct inode *ip = inode;
struct smack_known *isp;
struct inode_smack *ispp;
- size_t label_len;
char *label = NULL;

if (strcmp(name, XATTR_SMACK_SUFFIX) == 0) {
@@ -1594,15 +1594,15 @@ static int smack_inode_getsecurity(struct mnt_idmap *idmap,
if (!label)
label = isp->smk_known;

- label_len = strlen(label);
-
if (alloc) {
*buffer = kstrdup(label, GFP_KERNEL);
if (*buffer == NULL)
return -ENOMEM;
}

- return label_len;
+ *len = strlen(label);
+
+ return 0;
}

--
2.30.2