[v12,03/11] proc: Use lsmids instead of lsm names for attrs

Message ID	20230629195535.2590-4-casey@schaufler-ca.com (mailing list archive)
State	Changes Requested
Delegated to:	Paul Moore
Headers	show Return-Path: <linux-security-module-owner@vger.kernel.org> From: Casey Schaufler <casey@schaufler-ca.com> To: casey@schaufler-ca.com, paul@paul-moore.com, linux-security-module@vger.kernel.org Cc: jmorris@namei.org, serge@hallyn.com, keescook@chromium.org, john.johansen@canonical.com, penguin-kernel@i-love.sakura.ne.jp, stephen.smalley.work@gmail.com, linux-kernel@vger.kernel.org, linux-api@vger.kernel.org, mic@digikod.net, linux-fsdevel@vger.kernel.org Subject: [PATCH v12 03/11] proc: Use lsmids instead of lsm names for attrs Date: Thu, 29 Jun 2023 12:55:27 -0700 Message-Id: <20230629195535.2590-4-casey@schaufler-ca.com> In-Reply-To: <20230629195535.2590-1-casey@schaufler-ca.com> References: <20230629195535.2590-1-casey@schaufler-ca.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	LSM: Three basic syscalls \| expand [v12,00/11] LSM: Three basic syscalls [v12,01/11] LSM: Identify modules by more than name [v12,02/11] LSM: Maintain a table of LSM attribute data [v12,03/11] proc: Use lsmids instead of lsm names for attrs [v12,04/11] LSM: syscalls for current process attributes [v12,05/11] LSM: Create lsm_list_modules system call [v12,06/11] LSM: wireup Linux Security Module syscalls [v12,07/11] LSM: Helpers for attribute names and filling lsm_ctx [v12,08/11] Smack: implement setselfattr and getselfattr hooks [v12,09/11] AppArmor: Add selfattr hooks [v12,10/11] SELinux: Add selfattr hooks [v12,11/11] LSM: selftests for Linux Security Module syscalls

Message ID

20230629195535.2590-4-casey@schaufler-ca.com (mailing list archive)

State

Changes Requested

Delegated to:

Paul Moore

Headers

From: Casey Schaufler <casey@schaufler-ca.com>
To: casey@schaufler-ca.com, paul@paul-moore.com,
        linux-security-module@vger.kernel.org
Cc: jmorris@namei.org, serge@hallyn.com, keescook@chromium.org,
        john.johansen@canonical.com, penguin-kernel@i-love.sakura.ne.jp,
        stephen.smalley.work@gmail.com, linux-kernel@vger.kernel.org,
        linux-api@vger.kernel.org, mic@digikod.net,
        linux-fsdevel@vger.kernel.org
Subject: [PATCH v12 03/11] proc: Use lsmids instead of lsm names for attrs
Date: Thu, 29 Jun 2023 12:55:27 -0700
Message-Id: <20230629195535.2590-4-casey@schaufler-ca.com>
In-Reply-To: <20230629195535.2590-1-casey@schaufler-ca.com>
References: <20230629195535.2590-1-casey@schaufler-ca.com>
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

Series

LSM: Three basic syscalls | expand

Commit Message

Casey Schaufler June 29, 2023, 7:55 p.m. UTC

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>
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

Mickaël Salaün July 11, 2023, 3:36 p.m. UTC | #1

On 29/06/2023 21:55, 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>
> Cc: linux-fsdevel@vger.kernel.org

Reviewed-by: Mickaël Salaün <mic@digikod.net>

> ---
>   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(&current->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 569b1d8ab002..945101b0d404 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 5a699e47478b..d942b0c8e32f 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);
>   	}

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(&current->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 569b1d8ab002..945101b0d404 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 5a699e47478b..d942b0c8e32f 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);
 	}

[v12,03/11] proc: Use lsmids instead of lsm names for attrs

Commit Message

Comments

Patch