[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CALCETrX80OeMormSV0Uyr4WhFdp-=yTphQ3s1Su_=zpR-GR_zw@mail.gmail.com>
Date: Wed, 20 May 2015 19:10:20 -0700
From: Andy Lutomirski <luto@...capital.net>
To: Calvin Owens <calvinowens@...com>
Cc: Andrew Morton <akpm@...ux-foundation.org>,
Alexey Dobriyan <adobriyan@...il.com>,
"Eric W. Biederman" <ebiederm@...ssion.com>,
Al Viro <viro@...iv.linux.org.uk>,
Miklos Szeredi <miklos@...redi.hu>,
Zefan Li <lizefan@...wei.com>, Oleg Nesterov <oleg@...hat.com>,
Joe Perches <joe@...ches.com>,
David Howells <dhowells@...hat.com>,
"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
kernel-team@...com, Kees Cook <keescook@...omium.org>,
"Kirill A. Shutemov" <kirill@...temov.name>
Subject: Re: [PATCH v5] procfs: Always expose /proc/<pid>/map_files/ and make
it readable
On Wed, May 20, 2015 at 6:52 PM, Calvin Owens <calvinowens@...com> wrote:
> On Tuesday 05/19 at 11:04 -0700, Andy Lutomirski wrote:
>> On Mon, May 18, 2015 at 8:10 PM, Calvin Owens <calvinowens@...com> wrote:
>> > Currently, /proc/<pid>/map_files/ is restricted to CAP_SYS_ADMIN, and
>> > is only exposed if CONFIG_CHECKPOINT_RESTORE is set. This interface is
>> > very useful for enumerating the files mapped into a process when the
>> > more verbose information in /proc/<pid>/maps is not needed. It also
>> > allows access to file descriptors for files that have been deleted and
>> > closed but are still mmapped into a process, which can be very useful
>> > for introspection and debugging.
>> >
>> > This patch moves the folder out from behind CHECKPOINT_RESTORE, and
>>
>> I'm fine with this.
>>
>> > removes the CAP_SYS_ADMIN restrictions. With that change alone,
>> > following the links would have required PTRACE_MODE_READ like the
>> > links in /proc/<pid>/fd/*.
>>
>> I'm still not at all convinced that this is safe. Here are a few ways
>> that it could have unintended consequences:
>>
>> 1. Mmap a dma-buf and then open /proc/self/map_files/addr. You get an
>> fd pointing at a different inode than you mapped. (kdbus would have
>> the same problem if it were merged.)
>>
>> 2. Open a file with O_RDONLY, mmap it with PROT_READ, close the file,
>> then open /proc/self/map_files/addr with O_RDWR. I don't see anything
>> preventing that from succeeding.
>
> Hmm, that's a good point: it lets you bypass the permission checks on
> all the path components you would normally walk through to get to the
> file. But it still only works if you actually have permission to open
> the file in question for writing.
But you might not still have that permission.
>
> Also, this is already how the /proc/N/fd/* symlinks work, isn't it?
Yes, but only for files that are open. Also, I hope to fix that some day.
>
>> 3. Open a file, mmap it, close the fd, chroot, drop privileges, open
>> /proc/self/map_files/addr, then call ftruncate.
>
> This doesn't work unless the privileges you dropped to actually allow
> you to open the mmapped file for writing. It's really the same
> fundamental problem as (2), where you're allowing direct access to a
> file without trying to walk the path down to it, right?
Yes, although I can imagine this actually happening. Also, there's issue #1.
>
>> So NAK as-is, I think.
>
> Limiting ->follow_link() to CAP_SYS_ADMIN wouldn't affect anything I
> imagine using this interface for (see below), so I have no problem with
> putting that back in. I think that would alleviate all your concerns
> above, right?
I think so. You could still maybe do awful things due to #1, but at
least you'd have to be privileged.
>
> (That said, I don't think it makes sense to limit readdir() or
> readlink() on map_files/* to CAP_SYS_ADMIN, since that alone is a subset
> of what you can get from /proc/N/maps.)
Agreed.
>
>> Fixing #1 would involve changing the way mmap works, I think. Fixing
>> #2 would require similar infrastructure to what we'd need to fix the
>> existing /proc/pid/fd mode holes. I have no clue how to even approach
>> fixing #3.
>>
>> What's the use case of this patch?
>
> The biggest use case: it enables you to stat() files that have been
> deleted but are still mapped by some process.
>
> This enables a much quicker and more accurate answer to the question
> "How much disk space is being consumed by files that are deleted but
> still mapped?" than is currently possible.
>
> It also allows you to know how much space a specific mapped-but-deleted
> file is using on a specific filesystem, which is currently impossible
> from userspace AFAIK.
Seems reasonable.
It might be nice to have a general interface for enumerating
deleted-but-still-in-use files on a filesystem some day, too.
--Andy
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists