| Message ID | 20200715144954.1387760-5-areber@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | capabilities: Introduce CAP_CHECKPOINT_RESTORE | expand |
On Wed, Jul 15, 2020 at 04:49:52PM +0200, Adrian Reber wrote: > Opening files in /proc/pid/map_files when the current user is > CAP_CHECKPOINT_RESTORE capable in the root namespace is useful for > checkpointing and restoring to recover files that are unreachable via > the file system such as deleted files, or memfd files. > > Signed-off-by: Adrian Reber <areber@redhat.com> > Signed-off-by: Nicolas Viennot <Nicolas.Viennot@twosigma.com> I still have a plan to make this code been usable without capabilities requirements but due to lack of spare time for deep investigation this won't happen anytime soon. Thus the patch looks OK to me, fwiw Reviewed-by: Cyrill Gorcunov <gorcunov@gmail.com>
On Wed, Jul 15, 2020 at 04:49:52PM +0200, Adrian Reber wrote: > Opening files in /proc/pid/map_files when the current user is > CAP_CHECKPOINT_RESTORE capable in the root namespace is useful for > checkpointing and restoring to recover files that are unreachable via > the file system such as deleted files, or memfd files. > > Signed-off-by: Adrian Reber <areber@redhat.com> > Signed-off-by: Nicolas Viennot <Nicolas.Viennot@twosigma.com> > --- > fs/proc/base.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/fs/proc/base.c b/fs/proc/base.c > index 65893686d1f1..cada783f229e 100644 > --- a/fs/proc/base.c > +++ b/fs/proc/base.c > @@ -2194,16 +2194,16 @@ struct map_files_info { > }; > > /* > - * Only allow CAP_SYS_ADMIN to follow the links, due to concerns about how the > - * symlinks may be used to bypass permissions on ancestor directories in the > - * path to the file in question. > + * Only allow CAP_SYS_ADMIN and CAP_CHECKPOINT_RESTORE to follow the links, due > + * to concerns about how the symlinks may be used to bypass permissions on > + * ancestor directories in the path to the file in question. > */ > static const char * > proc_map_files_get_link(struct dentry *dentry, > struct inode *inode, > struct delayed_call *done) > { > - if (!capable(CAP_SYS_ADMIN)) > + if (!capable(CAP_SYS_ADMIN) || !capable(CAP_CHECKPOINT_RESTORE)) So right now, when I'd do git grep checkpoint_restore_ns_capable I'd not hit that codepath which isn't great. So I'd suggest to use: if (!checkpoint_restore_ns_capable(&init_user_ns)) at the end of the day, capable(<cap>) just calls ns_capable(&init_user_ns, <cap>) anyway. Thanks! Christian
diff --git a/fs/proc/base.c b/fs/proc/base.c index 65893686d1f1..cada783f229e 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c @@ -2194,16 +2194,16 @@ struct map_files_info { }; /* - * Only allow CAP_SYS_ADMIN to follow the links, due to concerns about how the - * symlinks may be used to bypass permissions on ancestor directories in the - * path to the file in question. + * Only allow CAP_SYS_ADMIN and CAP_CHECKPOINT_RESTORE to follow the links, due + * to concerns about how the symlinks may be used to bypass permissions on + * ancestor directories in the path to the file in question. */ static const char * proc_map_files_get_link(struct dentry *dentry, struct inode *inode, struct delayed_call *done) { - if (!capable(CAP_SYS_ADMIN)) + if (!capable(CAP_SYS_ADMIN) || !capable(CAP_CHECKPOINT_RESTORE)) return ERR_PTR(-EPERM); return proc_pid_get_link(dentry, inode, done);