Thread (15 messages) 15 messages, 6 authors, 2020-06-10

Re: [PATCH v2 1/3] capabilities: Introduce CAP_CHECKPOINT_RESTORE

From: Cyrill Gorcunov <hidden>
Date: 2020-06-09 21:28:28
Also in: linux-fsdevel, lkml, selinux

On Tue, Jun 09, 2020 at 08:09:49PM +0000, Nicolas Viennot wrote:
quoted
quoted
 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);
quoted
First of all -- sorry for late reply. You know, looking into this code more I think
this CAP_SYS_ADMIN is simply wrong: for example I can't even fetch links for /proc/self/map_files.
Still /proc/$pid/maps (which as well points to the files opened) test for ptrace-read permission.
I think we need ptrace-may-attach test here instead of these capabilities (if I can attach to
a process I can read any data needed, including the content of the mapped files, if only
I'm not missing something obvious).
Nikolas, could you please split the text lines next time, I've had to add newlines into reply manually :)
Currently /proc/pid/map_files/* have exactly the same permission checks as /proc/pid/fd/*, with the exception
of the extra CAP_SYS_ADMIN check. The check originated from the following discussions where 3 security issues are discussed:
http://lkml.iu.edu/hypermail/linux/kernel/1505.2/02524.html
http://lkml.iu.edu/hypermail/linux/kernel/1505.2/04030.html

From what I understand, the extra CAP_SYS_ADMIN comes from the following issues:
1. Being able to open dma-buf / kdbus region (referred in the referenced email as problem #1).
I don't fully understand what the dangers are, but perhaps we could do CAP_SYS_ADMIN check
only for such dangerous files, as opposed to all files.
As far as I remember we only need to read the content of mmap'ed files and if I've ptrace-attach
permission we aready can inject own code into a process and read anything we wish. That said we probably
should fixup this interface like -- test for open mode and if it is read only then ptrace-attach
should be enough, if it is write mode -- then we require being node's admin instead of just adding
a new capability here. And thanks a huge for mail reference, I'll take a look once time permit.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help