On Tue, May 25, 2021 at 9:37 PM Kees Cook <keescook@xxxxxxxxxxxx> wrote: > Fix another "confused deputy" weakness[1]. Writes to /proc/$pid/attr/ > files need to check the opener credentials, since these fds do not > transition state across execve(). Without this, it is possible to > trick another process (which may have different credentials) to write > to its own /proc/$pid/attr/ files, leading to unexpected and possibly > exploitable behaviors. > > [1] https://www.kernel.org/doc/html/latest/security/credentials.html?highlight=confused#open-file-credentials > > Fixes: 1da177e4c3f41 ("Linux-2.6.12-rc2") > Cc: stable@xxxxxxxxxxxxxxx > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx> > --- > fs/proc/base.c | 4 ++++ > 1 file changed, 4 insertions(+) > > diff --git a/fs/proc/base.c b/fs/proc/base.c > index 3851bfcdba56..58bbf334265b 100644 > --- a/fs/proc/base.c > +++ b/fs/proc/base.c > @@ -2703,6 +2703,10 @@ static ssize_t proc_pid_attr_write(struct file * file, const char __user * buf, > void *page; > int rv; > > + /* A task may only write when it was the opener. */ > + if (file->f_cred != current_real_cred()) > + return -EPERM; With this, if a task forks, the child will then still be able to open its parent's /proc/$pid/attr/current and trick the parent into writing to that, right? Is that acceptable? If not, the ->open handler should probably also check for "current->thread_pid == proc_pid(inode)", or something like that?
