Re: [PATCH 2/2] proc: use generic setattr() for /proc/$PID/net

[Christian Brauner]

On Sat, Jun 24, 2023 at 12:30:47PM +0200, Thomas Weißschuh wrote:
> All other files in /proc/$PID/ use proc_setattr().
> 
> Not using it allows the usage of chmod() on /proc/$PID/net, even on
> other processes owned by the same user.
> The same would probably also be true for other attributes to be changed.
> 
> As this technically represents an ABI change it is not marked for
> stable so any unlikely regressions are caught during a full release cycle.
> 
> Fixes: e9720acd728a ("[NET]: Make /proc/net a symlink on /proc/self/net (v3)")
> Signed-off-by: Thomas Weißschuh <linux@xxxxxxxxxxxxxx>
> ---
>  fs/proc/proc_net.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/fs/proc/proc_net.c b/fs/proc/proc_net.c
> index a0c0419872e3..78f9e6b469c0 100644
> --- a/fs/proc/proc_net.c
> +++ b/fs/proc/proc_net.c
> @@ -321,6 +321,7 @@ static int proc_tgid_net_getattr(struct mnt_idmap *idmap,
>  const struct inode_operations proc_net_inode_operations = {
>  	.lookup		= proc_tgid_net_lookup,
>  	.getattr	= proc_tgid_net_getattr,
> +	.setattr        = proc_setattr,
>  };
>  
>  static int proc_tgid_net_readdir(struct file *file, struct dir_context *ctx)

So your concern really is specifically about /proc/$pid/net itself as
that's owned by the user and thus the user itself can chmod it and thus
also restrict access for other processess running with the same uid:

chmod 0000 /proc/1234/net
ls -al /proc/self/net
ls: cannot open directory '/proc/self/net/': Permission denied

Yeah, it's not a huge deal but it's arguably a bug especially since the
original commit from 2006 that introduced proc_setattr() was clear that
it should apply to anything beneath /proc/<pid>/ owned by the user.

So I agree and we should probably try and have the same behavior for
/proc/$pid/net as well. We can see if that breaks something.