[PATCHES] struct path constification

[PATCHES] struct path constification

[Al Viro]

	This is mostly whack-a-mole stuff - a bunch of places
are passing struct path pointers around, without bothering
to mark them const, even though they are not going to try
and modify the contents of struct path.

	It's a bad practice, since there are invariants along
the lines of "file->f_path stays unchanged open-to-release"
and verifying those can get very unpleasant when you are
forced to take detours down the long call chains that could've
been avoided.

	Patches in that pile are independent from each other
and if anyone wants to grab some of them into subsystem's
tree - just say so; I'll be happy to exclude those from the
vfs.git branch if they go into another tree.

	Currently they are in vfs.git#work.path; individual
patches in followups.

Subject: [PATCH 01/11] ->getprocattr(): attribute name is const char *, TYVM...

[Al Viro]

cast of ->d_name.name to char * is completely wrong - nothing is
allowed to modify its contents.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/proc/base.c                | 2 +-
 include/linux/lsm_hook_defs.h | 2 +-
 include/linux/security.h      | 4 ++--
 security/apparmor/lsm.c       | 2 +-
 security/security.c           | 4 ++--
 security/selinux/hooks.c      | 2 +-
 security/smack/smack_lsm.c    | 2 +-
 7 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/fs/proc/base.c b/fs/proc/base.c
index 93f7e3d971e4..e347b8ce140c 100644
--- a/fs/proc/base.c
+++ b/fs/proc/base.c
@@ -2728,7 +2728,7 @@ static ssize_t proc_pid_attr_read(struct file * file, char __user * buf,
 		return -ESRCH;
 
 	length = security_getprocattr(task, PROC_I(inode)->op.lsm,
-				      (char*)file->f_path.dentry->d_name.name,
+				      file->f_path.dentry->d_name.name,
 				      &p);
 	put_task_struct(task);
 	if (length > 0)
diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
index 806448173033..03360d27bedf 100644
--- a/include/linux/lsm_hook_defs.h
+++ b/include/linux/lsm_hook_defs.h
@@ -253,7 +253,7 @@ LSM_HOOK(int, 0, sem_semop, struct kern_ipc_perm *perm, struct sembuf *sops,
 LSM_HOOK(int, 0, netlink_send, struct sock *sk, struct sk_buff *skb)
 LSM_HOOK(void, LSM_RET_VOID, d_instantiate, struct dentry *dentry,
 	 struct inode *inode)
-LSM_HOOK(int, -EINVAL, getprocattr, struct task_struct *p, char *name,
+LSM_HOOK(int, -EINVAL, getprocattr, struct task_struct *p, const char *name,
 	 char **value)
 LSM_HOOK(int, -EINVAL, setprocattr, const char *name, void *value, size_t size)
 LSM_HOOK(int, 0, ismaclabel, const char *name)
diff --git a/include/linux/security.h b/include/linux/security.h
index 1bc362cb413f..93488c01d9bd 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -461,7 +461,7 @@ 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, char *name,
+int security_getprocattr(struct task_struct *p, const char *lsm, const char *name,
 			 char **value);
 int security_setprocattr(const char *lsm, const char *name, void *value,
 			 size_t size);
@@ -1301,7 +1301,7 @@ static inline void security_d_instantiate(struct dentry *dentry,
 { }
 
 static inline int security_getprocattr(struct task_struct *p, const char *lsm,
-				       char *name, char **value)
+				       const char *name, char **value)
 {
 	return -EINVAL;
 }
diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
index e29cade7b662..f56070270c69 100644
--- a/security/apparmor/lsm.c
+++ b/security/apparmor/lsm.c
@@ -614,7 +614,7 @@ static int apparmor_sb_pivotroot(const struct path *old_path,
 	return error;
 }
 
-static int apparmor_getprocattr(struct task_struct *task, char *name,
+static int apparmor_getprocattr(struct task_struct *task, const char *name,
 				char **value)
 {
 	int error = -ENOENT;
diff --git a/security/security.c b/security/security.c
index 14d30fec8a00..d8227531e2fd 100644
--- a/security/security.c
+++ b/security/security.c
@@ -2057,8 +2057,8 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode)
 }
 EXPORT_SYMBOL(security_d_instantiate);
 
-int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
-				char **value)
+int security_getprocattr(struct task_struct *p, const char *lsm,
+			 const char *name, char **value)
 {
 	struct security_hook_list *hp;
 
diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
index 79573504783b..c8168d19fb96 100644
--- a/security/selinux/hooks.c
+++ b/security/selinux/hooks.c
@@ -6327,7 +6327,7 @@ static void selinux_d_instantiate(struct dentry *dentry, struct inode *inode)
 }
 
 static int selinux_getprocattr(struct task_struct *p,
-			       char *name, char **value)
+			       const char *name, char **value)
 {
 	const struct task_security_struct *__tsec;
 	u32 sid;
diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
index 001831458fa2..434b348d8fcd 100644
--- a/security/smack/smack_lsm.c
+++ b/security/smack/smack_lsm.c
@@ -3479,7 +3479,7 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
  *
  * Returns the length of the smack label or an error code
  */
-static int smack_getprocattr(struct task_struct *p, char *name, char **value)
+static int smack_getprocattr(struct task_struct *p, const char *name, char **value)
 {
 	struct smack_known *skp = smk_of_task_struct_obj(p);
 	char *cp;
-- 
2.30.2

Re: Subject: [PATCH 01/11] ->getprocattr(): attribute name is const char *, TYVM...

[Paul Moore]

On Sat, Aug 20, 2022 at 2:12 PM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
> cast of ->d_name.name to char * is completely wrong - nothing is
> allowed to modify its contents.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
> ---
>  fs/proc/base.c                | 2 +-
>  include/linux/lsm_hook_defs.h | 2 +-
>  include/linux/security.h      | 4 ++--
>  security/apparmor/lsm.c       | 2 +-
>  security/security.c           | 4 ++--
>  security/selinux/hooks.c      | 2 +-
>  security/smack/smack_lsm.c    | 2 +-
>  7 files changed, 9 insertions(+), 9 deletions(-)

Thanks Al.  Based on your other email it sounds like you are going to
send these up to Linus, which is fine by me, but if that changes let
me know and I'll make sure this patch gets sent up.

Acked-by: Paul Moore <paul@paul-moore.com>

-- 
paul-moore.com

Re: Subject: [PATCH 01/11] ->getprocattr(): attribute name is const char *, TYVM...

[Casey Schaufler]

On 8/20/2022 11:11 AM, Al Viro wrote:
> cast of ->d_name.name to char * is completely wrong - nothing is
> allowed to modify its contents.
>
> Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>

Acked-by: Casey Schaufler <casey@schaufler-ca.com>

Thank you for the fix. Please feel free to submit this
directly.

> ---
>  fs/proc/base.c                | 2 +-
>  include/linux/lsm_hook_defs.h | 2 +-
>  include/linux/security.h      | 4 ++--
>  security/apparmor/lsm.c       | 2 +-
>  security/security.c           | 4 ++--
>  security/selinux/hooks.c      | 2 +-
>  security/smack/smack_lsm.c    | 2 +-
>  7 files changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/fs/proc/base.c b/fs/proc/base.c
> index 93f7e3d971e4..e347b8ce140c 100644
> --- a/fs/proc/base.c
> +++ b/fs/proc/base.c
> @@ -2728,7 +2728,7 @@ static ssize_t proc_pid_attr_read(struct file * file, char __user * buf,
>  		return -ESRCH;
>  
>  	length = security_getprocattr(task, PROC_I(inode)->op.lsm,
> -				      (char*)file->f_path.dentry->d_name.name,
> +				      file->f_path.dentry->d_name.name,
>  				      &p);
>  	put_task_struct(task);
>  	if (length > 0)
> diff --git a/include/linux/lsm_hook_defs.h b/include/linux/lsm_hook_defs.h
> index 806448173033..03360d27bedf 100644
> --- a/include/linux/lsm_hook_defs.h
> +++ b/include/linux/lsm_hook_defs.h
> @@ -253,7 +253,7 @@ LSM_HOOK(int, 0, sem_semop, struct kern_ipc_perm *perm, struct sembuf *sops,
>  LSM_HOOK(int, 0, netlink_send, struct sock *sk, struct sk_buff *skb)
>  LSM_HOOK(void, LSM_RET_VOID, d_instantiate, struct dentry *dentry,
>  	 struct inode *inode)
> -LSM_HOOK(int, -EINVAL, getprocattr, struct task_struct *p, char *name,
> +LSM_HOOK(int, -EINVAL, getprocattr, struct task_struct *p, const char *name,
>  	 char **value)
>  LSM_HOOK(int, -EINVAL, setprocattr, const char *name, void *value, size_t size)
>  LSM_HOOK(int, 0, ismaclabel, const char *name)
> diff --git a/include/linux/security.h b/include/linux/security.h
> index 1bc362cb413f..93488c01d9bd 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -461,7 +461,7 @@ 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, char *name,
> +int security_getprocattr(struct task_struct *p, const char *lsm, const char *name,
>  			 char **value);
>  int security_setprocattr(const char *lsm, const char *name, void *value,
>  			 size_t size);
> @@ -1301,7 +1301,7 @@ static inline void security_d_instantiate(struct dentry *dentry,
>  { }
>  
>  static inline int security_getprocattr(struct task_struct *p, const char *lsm,
> -				       char *name, char **value)
> +				       const char *name, char **value)
>  {
>  	return -EINVAL;
>  }
> diff --git a/security/apparmor/lsm.c b/security/apparmor/lsm.c
> index e29cade7b662..f56070270c69 100644
> --- a/security/apparmor/lsm.c
> +++ b/security/apparmor/lsm.c
> @@ -614,7 +614,7 @@ static int apparmor_sb_pivotroot(const struct path *old_path,
>  	return error;
>  }
>  
> -static int apparmor_getprocattr(struct task_struct *task, char *name,
> +static int apparmor_getprocattr(struct task_struct *task, const char *name,
>  				char **value)
>  {
>  	int error = -ENOENT;
> diff --git a/security/security.c b/security/security.c
> index 14d30fec8a00..d8227531e2fd 100644
> --- a/security/security.c
> +++ b/security/security.c
> @@ -2057,8 +2057,8 @@ void security_d_instantiate(struct dentry *dentry, struct inode *inode)
>  }
>  EXPORT_SYMBOL(security_d_instantiate);
>  
> -int security_getprocattr(struct task_struct *p, const char *lsm, char *name,
> -				char **value)
> +int security_getprocattr(struct task_struct *p, const char *lsm,
> +			 const char *name, char **value)
>  {
>  	struct security_hook_list *hp;
>  
> diff --git a/security/selinux/hooks.c b/security/selinux/hooks.c
> index 79573504783b..c8168d19fb96 100644
> --- a/security/selinux/hooks.c
> +++ b/security/selinux/hooks.c
> @@ -6327,7 +6327,7 @@ static void selinux_d_instantiate(struct dentry *dentry, struct inode *inode)
>  }
>  
>  static int selinux_getprocattr(struct task_struct *p,
> -			       char *name, char **value)
> +			       const char *name, char **value)
>  {
>  	const struct task_security_struct *__tsec;
>  	u32 sid;
> diff --git a/security/smack/smack_lsm.c b/security/smack/smack_lsm.c
> index 001831458fa2..434b348d8fcd 100644
> --- a/security/smack/smack_lsm.c
> +++ b/security/smack/smack_lsm.c
> @@ -3479,7 +3479,7 @@ static void smack_d_instantiate(struct dentry *opt_dentry, struct inode *inode)
>   *
>   * Returns the length of the smack label or an error code
>   */
> -static int smack_getprocattr(struct task_struct *p, char *name, char **value)
> +static int smack_getprocattr(struct task_struct *p, const char *name, char **value)
>  {
>  	struct smack_known *skp = smk_of_task_struct_obj(p);
>  	char *cp;

Re: [PATCHES] struct path constification

[Christian Brauner]

On Sat, Aug 20, 2022 at 07:10:38PM +0100, Al Viro wrote:
> 	This is mostly whack-a-mole stuff - a bunch of places
> are passing struct path pointers around, without bothering
> to mark them const, even though they are not going to try
> and modify the contents of struct path.
> 
> 	It's a bad practice, since there are invariants along
> the lines of "file->f_path stays unchanged open-to-release"
> and verifying those can get very unpleasant when you are
> forced to take detours down the long call chains that could've
> been avoided.
> 
> 	Patches in that pile are independent from each other
> and if anyone wants to grab some of them into subsystem's
> tree - just say so; I'll be happy to exclude those from the
> vfs.git branch if they go into another tree.
> 
> 	Currently they are in vfs.git#work.path; individual
> patches in followups.
> 

Looks good,
Reviewed-by: Christian Brauner (Microsoft) <brauner@kernel.org>