[v13,03/11] proc: Use lsmids instead of lsm names for attrs
Commit Message
Use the LSM ID number instead of the LSM name to identify which
security module's attibute data should be shown in /proc/self/attr.
The security_[gs]etprocattr() functions have been changed to expect
the LSM ID. The change from a string comparison to an integer comparison
in these functions will provide a minor performance improvement.
Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
Reviewed-by: Kees Cook <keescook@chromium.org>
Reviewed-by: Serge Hallyn <serge@hallyn.com>
Reviewed-by: Mickael Salaun <mic@digikod.net>
Cc: linux-fsdevel@vger.kernel.org
---
fs/proc/base.c | 29 +++++++++++++++--------------
fs/proc/internal.h | 2 +-
include/linux/security.h | 11 +++++------
security/security.c | 15 +++++++--------
4 files changed, 28 insertions(+), 29 deletions(-)
Comments
On 8/2/23 10:44, Casey Schaufler wrote:
> Use the LSM ID number instead of the LSM name to identify which
> security module's attibute data should be shown in /proc/self/attr.
> The security_[gs]etprocattr() functions have been changed to expect
> the LSM ID. The change from a string comparison to an integer comparison
> in these functions will provide a minor performance improvement.
>
> Signed-off-by: Casey Schaufler <casey@schaufler-ca.com>
> Reviewed-by: Kees Cook <keescook@chromium.org>
> Reviewed-by: Serge Hallyn <serge@hallyn.com>
> Reviewed-by: Mickael Salaun <mic@digikod.net>
> Cc: linux-fsdevel@vger.kernel.org
Reviewed-by: John Johansen <john.johansen@canonical.com>
> ---
> fs/proc/base.c | 29 +++++++++++++++--------------
> fs/proc/internal.h | 2 +-
> include/linux/security.h | 11 +++++------
> security/security.c | 15 +++++++--------
> 4 files changed, 28 insertions(+), 29 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 05452c3b9872..f999bb5c497b 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -97,6 +97,7 @@
> #include <linux/resctrl.h>
> #include <linux/cn_proc.h>
> #include <linux/ksm.h>
> +#include <uapi/linux/lsm.h>
> #include <trace/events/oom.h>
> #include "internal.h"
> #include "fd.h"
> @@ -146,10 +147,10 @@ struct pid_entry {
> NOD(NAME, (S_IFREG|(MODE)), \
> NULL, &proc_single_file_operations, \
> { .proc_show = show } )
> -#define ATTR(LSM, NAME, MODE) \
> +#define ATTR(LSMID, NAME, MODE) \
> NOD(NAME, (S_IFREG|(MODE)), \
> NULL, &proc_pid_attr_operations, \
> - { .lsm = LSM })
> + { .lsmid = LSMID })
>
> /*
> * Count the number of hardlinks for the pid_entry table, excluding the .
> @@ -2730,7 +2731,7 @@ static ssize_t proc_pid_attr_read(struct file * file, char __user * buf,
> if (!task)
> return -ESRCH;
>
> - length = security_getprocattr(task, PROC_I(inode)->op.lsm,
> + length = security_getprocattr(task, PROC_I(inode)->op.lsmid,
> file->f_path.dentry->d_name.name,
> &p);
> put_task_struct(task);
> @@ -2788,7 +2789,7 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
> if (rv < 0)
> goto out_free;
>
> - rv = security_setprocattr(PROC_I(inode)->op.lsm,
> + rv = security_setprocattr(PROC_I(inode)->op.lsmid,
> file->f_path.dentry->d_name.name, page,
> count);
> mutex_unlock(¤t->signal->cred_guard_mutex);
> @@ -2837,27 +2838,27 @@ static const struct inode_operations proc_##LSM##_attr_dir_inode_ops = { \
>
> #ifdef CONFIG_SECURITY_SMACK
> static const struct pid_entry smack_attr_dir_stuff[] = {
> - ATTR("smack", "current", 0666),
> + ATTR(LSM_ID_SMACK, "current", 0666),
> };
> LSM_DIR_OPS(smack);
> #endif
>
> #ifdef CONFIG_SECURITY_APPARMOR
> static const struct pid_entry apparmor_attr_dir_stuff[] = {
> - ATTR("apparmor", "current", 0666),
> - ATTR("apparmor", "prev", 0444),
> - ATTR("apparmor", "exec", 0666),
> + ATTR(LSM_ID_APPARMOR, "current", 0666),
> + ATTR(LSM_ID_APPARMOR, "prev", 0444),
> + ATTR(LSM_ID_APPARMOR, "exec", 0666),
> };
> LSM_DIR_OPS(apparmor);
> #endif
>
> static const struct pid_entry attr_dir_stuff[] = {
> - ATTR(NULL, "current", 0666),
> - ATTR(NULL, "prev", 0444),
> - ATTR(NULL, "exec", 0666),
> - ATTR(NULL, "fscreate", 0666),
> - ATTR(NULL, "keycreate", 0666),
> - ATTR(NULL, "sockcreate", 0666),
> + ATTR(LSM_ID_UNDEF, "current", 0666),
> + ATTR(LSM_ID_UNDEF, "prev", 0444),
> + ATTR(LSM_ID_UNDEF, "exec", 0666),
> + ATTR(LSM_ID_UNDEF, "fscreate", 0666),
> + ATTR(LSM_ID_UNDEF, "keycreate", 0666),
> + ATTR(LSM_ID_UNDEF, "sockcreate", 0666),
> #ifdef CONFIG_SECURITY_SMACK
> DIR("smack", 0555,
> proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
> diff --git a/fs/proc/internal.h b/fs/proc/internal.h
> index 9dda7e54b2d0..a889d9ef9584 100644
> --- a/fs/proc/internal.h
> +++ b/fs/proc/internal.h
> @@ -92,7 +92,7 @@ union proc_op {
> int (*proc_show)(struct seq_file *m,
> struct pid_namespace *ns, struct pid *pid,
> struct task_struct *task);
> - const char *lsm;
> + int lsmid;
> };
>
> struct proc_inode {
> diff --git a/include/linux/security.h b/include/linux/security.h
> index a20a4ceda6d9..b5fd3f7f4cd3 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -470,10 +470,9 @@ int security_sem_semctl(struct kern_ipc_perm *sma, int cmd);
> int security_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops,
> unsigned nsops, int alter);
> void security_d_instantiate(struct dentry *dentry, struct inode *inode);
> -int security_getprocattr(struct task_struct *p, const char *lsm, const char *name,
> +int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
> char **value);
> -int security_setprocattr(const char *lsm, const char *name, void *value,
> - size_t size);
> +int security_setprocattr(int lsmid, const char *name, void *value, size_t size);
> int security_netlink_send(struct sock *sk, struct sk_buff *skb);
> int security_ismaclabel(const char *name);
> int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
> @@ -1332,14 +1331,14 @@ static inline void security_d_instantiate(struct dentry *dentry,
> struct inode *inode)
> { }
>
> -static inline int security_getprocattr(struct task_struct *p, const char *lsm,
> +static inline int security_getprocattr(struct task_struct *p, int lsmid,
> const char *name, char **value)
> {
> return -EINVAL;
> }
>
> -static inline int security_setprocattr(const char *lsm, char *name,
> - void *value, size_t size)
> +static inline int security_setprocattr(int lsmid, char *name, void *value,
> + size_t size)
> {
> return -EINVAL;
> }
> diff --git a/security/security.c b/security/security.c
> index 87b70a55a028..5e9cd548dd95 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -3801,7 +3801,7 @@ EXPORT_SYMBOL(security_d_instantiate);
> /**
> * security_getprocattr() - Read an attribute for a task
> * @p: the task
> - * @lsm: LSM name
> + * @lsmid: LSM identification
> * @name: attribute name
> * @value: attribute value
> *
> @@ -3809,13 +3809,13 @@ EXPORT_SYMBOL(security_d_instantiate);
> *
> * Return: Returns the length of @value on success, a negative value otherwise.
> */
> -int security_getprocattr(struct task_struct *p, const char *lsm,
> - const char *name, char **value)
> +int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
> + char **value)
> {
> struct security_hook_list *hp;
>
> hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
> - if (lsm != NULL && strcmp(lsm, hp->lsmid->name))
> + if (lsmid != 0 && lsmid != hp->lsmid->id)
> continue;
> return hp->hook.getprocattr(p, name, value);
> }
> @@ -3824,7 +3824,7 @@ int security_getprocattr(struct task_struct *p, const char *lsm,
>
> /**
> * security_setprocattr() - Set an attribute for a task
> - * @lsm: LSM name
> + * @lsmid: LSM identification
> * @name: attribute name
> * @value: attribute value
> * @size: attribute value size
> @@ -3834,13 +3834,12 @@ int security_getprocattr(struct task_struct *p, const char *lsm,
> *
> * Return: Returns bytes written on success, a negative value otherwise.
> */
> -int security_setprocattr(const char *lsm, const char *name, void *value,
> - size_t size)
> +int security_setprocattr(int lsmid, const char *name, void *value, size_t size)
> {
> struct security_hook_list *hp;
>
> hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
> - if (lsm != NULL && strcmp(lsm, hp->lsmid->name))
> + if (lsmid != 0 && lsmid != hp->lsmid->id)
> continue;
> return hp->hook.setprocattr(name, value, size);
> }
@@ -97,6 +97,7 @@
#include <linux/resctrl.h>
#include <linux/cn_proc.h>
#include <linux/ksm.h>
+#include <uapi/linux/lsm.h>
#include <trace/events/oom.h>
#include "internal.h"
#include "fd.h"
@@ -146,10 +147,10 @@ struct pid_entry {
NOD(NAME, (S_IFREG|(MODE)), \
NULL, &proc_single_file_operations, \
{ .proc_show = show } )
-#define ATTR(LSM, NAME, MODE) \
+#define ATTR(LSMID, NAME, MODE) \
NOD(NAME, (S_IFREG|(MODE)), \
NULL, &proc_pid_attr_operations, \
- { .lsm = LSM })
+ { .lsmid = LSMID })
/*
* Count the number of hardlinks for the pid_entry table, excluding the .
@@ -2730,7 +2731,7 @@ static ssize_t proc_pid_attr_read(struct file * file, char __user * buf,
if (!task)
return -ESRCH;
- length = security_getprocattr(task, PROC_I(inode)->op.lsm,
+ length = security_getprocattr(task, PROC_I(inode)->op.lsmid,
file->f_path.dentry->d_name.name,
&p);
put_task_struct(task);
@@ -2788,7 +2789,7 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf,
if (rv < 0)
goto out_free;
- rv = security_setprocattr(PROC_I(inode)->op.lsm,
+ rv = security_setprocattr(PROC_I(inode)->op.lsmid,
file->f_path.dentry->d_name.name, page,
count);
mutex_unlock(¤t->signal->cred_guard_mutex);
@@ -2837,27 +2838,27 @@ static const struct inode_operations proc_##LSM##_attr_dir_inode_ops = { \
#ifdef CONFIG_SECURITY_SMACK
static const struct pid_entry smack_attr_dir_stuff[] = {
- ATTR("smack", "current", 0666),
+ ATTR(LSM_ID_SMACK, "current", 0666),
};
LSM_DIR_OPS(smack);
#endif
#ifdef CONFIG_SECURITY_APPARMOR
static const struct pid_entry apparmor_attr_dir_stuff[] = {
- ATTR("apparmor", "current", 0666),
- ATTR("apparmor", "prev", 0444),
- ATTR("apparmor", "exec", 0666),
+ ATTR(LSM_ID_APPARMOR, "current", 0666),
+ ATTR(LSM_ID_APPARMOR, "prev", 0444),
+ ATTR(LSM_ID_APPARMOR, "exec", 0666),
};
LSM_DIR_OPS(apparmor);
#endif
static const struct pid_entry attr_dir_stuff[] = {
- ATTR(NULL, "current", 0666),
- ATTR(NULL, "prev", 0444),
- ATTR(NULL, "exec", 0666),
- ATTR(NULL, "fscreate", 0666),
- ATTR(NULL, "keycreate", 0666),
- ATTR(NULL, "sockcreate", 0666),
+ ATTR(LSM_ID_UNDEF, "current", 0666),
+ ATTR(LSM_ID_UNDEF, "prev", 0444),
+ ATTR(LSM_ID_UNDEF, "exec", 0666),
+ ATTR(LSM_ID_UNDEF, "fscreate", 0666),
+ ATTR(LSM_ID_UNDEF, "keycreate", 0666),
+ ATTR(LSM_ID_UNDEF, "sockcreate", 0666),
#ifdef CONFIG_SECURITY_SMACK
DIR("smack", 0555,
proc_smack_attr_dir_inode_ops, proc_smack_attr_dir_ops),
@@ -92,7 +92,7 @@ union proc_op {
int (*proc_show)(struct seq_file *m,
struct pid_namespace *ns, struct pid *pid,
struct task_struct *task);
- const char *lsm;
+ int lsmid;
};
struct proc_inode {
@@ -470,10 +470,9 @@ int security_sem_semctl(struct kern_ipc_perm *sma, int cmd);
int security_sem_semop(struct kern_ipc_perm *sma, struct sembuf *sops,
unsigned nsops, int alter);
void security_d_instantiate(struct dentry *dentry, struct inode *inode);
-int security_getprocattr(struct task_struct *p, const char *lsm, const char *name,
+int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
char **value);
-int security_setprocattr(const char *lsm, const char *name, void *value,
- size_t size);
+int security_setprocattr(int lsmid, const char *name, void *value, size_t size);
int security_netlink_send(struct sock *sk, struct sk_buff *skb);
int security_ismaclabel(const char *name);
int security_secid_to_secctx(u32 secid, char **secdata, u32 *seclen);
@@ -1332,14 +1331,14 @@ static inline void security_d_instantiate(struct dentry *dentry,
struct inode *inode)
{ }
-static inline int security_getprocattr(struct task_struct *p, const char *lsm,
+static inline int security_getprocattr(struct task_struct *p, int lsmid,
const char *name, char **value)
{
return -EINVAL;
}
-static inline int security_setprocattr(const char *lsm, char *name,
- void *value, size_t size)
+static inline int security_setprocattr(int lsmid, char *name, void *value,
+ size_t size)
{
return -EINVAL;
}
@@ -3801,7 +3801,7 @@ EXPORT_SYMBOL(security_d_instantiate);
/**
* security_getprocattr() - Read an attribute for a task
* @p: the task
- * @lsm: LSM name
+ * @lsmid: LSM identification
* @name: attribute name
* @value: attribute value
*
@@ -3809,13 +3809,13 @@ EXPORT_SYMBOL(security_d_instantiate);
*
* Return: Returns the length of @value on success, a negative value otherwise.
*/
-int security_getprocattr(struct task_struct *p, const char *lsm,
- const char *name, char **value)
+int security_getprocattr(struct task_struct *p, int lsmid, const char *name,
+ char **value)
{
struct security_hook_list *hp;
hlist_for_each_entry(hp, &security_hook_heads.getprocattr, list) {
- if (lsm != NULL && strcmp(lsm, hp->lsmid->name))
+ if (lsmid != 0 && lsmid != hp->lsmid->id)
continue;
return hp->hook.getprocattr(p, name, value);
}
@@ -3824,7 +3824,7 @@ int security_getprocattr(struct task_struct *p, const char *lsm,
/**
* security_setprocattr() - Set an attribute for a task
- * @lsm: LSM name
+ * @lsmid: LSM identification
* @name: attribute name
* @value: attribute value
* @size: attribute value size
@@ -3834,13 +3834,12 @@ int security_getprocattr(struct task_struct *p, const char *lsm,
*
* Return: Returns bytes written on success, a negative value otherwise.
*/
-int security_setprocattr(const char *lsm, const char *name, void *value,
- size_t size)
+int security_setprocattr(int lsmid, const char *name, void *value, size_t size)
{
struct security_hook_list *hp;
hlist_for_each_entry(hp, &security_hook_heads.setprocattr, list) {
- if (lsm != NULL && strcmp(lsm, hp->lsmid->name))
+ if (lsmid != 0 && lsmid != hp->lsmid->id)
continue;
return hp->hook.setprocattr(name, value, size);
}