| Message ID | 20230624-proc-net-setattr-v1-2-73176812adee@weissschuh.net (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 18e66ae67673f2c8df6f02428798b1355691f2a9 |
| Headers | show |
| Series | proc: proc_setattr for /proc/$PID/net | expand |
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@weissschuh.net> > --- > 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.
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)
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@weissschuh.net> --- fs/proc/proc_net.c | 1 + 1 file changed, 1 insertion(+)