[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH v2] linux-user: fix is_proc_myself to check the
From: |
Zach Riggle |
Subject: |
Re: [Qemu-devel] [PATCH v2] linux-user: fix is_proc_myself to check the paths via realpath |
Date: |
Fri, 10 Nov 2017 19:48:09 -0600 |
I wrote up a quick example to show that this should work specifically for
/proc/self/exe:
#define _GNU_SOURCE
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>
#include <unistd.h>
int main(int argc, char** argv) {
int fd = open("/proc/self/exe", O_NOFOLLOW | O_PATH);
system("ls -la /proc/$PPID/fd/");
}
*Zach Riggle*
On Fri, Nov 10, 2017 at 7:47 PM, Zach Riggle <address@hidden> wrote:
> Good catch. Relying on realpath() for *exe* does cause issues.
>
> A better general solution (which handles the "exe" case) is to use open(2)
> with O_PATH | O_NOFOLLOW for the candidate path (e.g. /proc/self/exe) and
> to do the same for the path we're testing along with readlink().
>
> If, in the process of link resolution via the readlink() loop, we end up
> with the same path as our candidate, we can return true. This avoids the
> need to rely on any libc implementation of realpath(), since we're just
> relying on the host kernel.
>
>
> *Zach Riggle*
>
> On Fri, Nov 10, 2017 at 5:44 PM, Laurent Vivier <address@hidden> wrote:
>
>> Le 25/10/2017 à 05:34, Zach Riggle a écrit :
>> > Previously, it was possible to get a handle to the "real" /proc/self/mem
>> > by creating a symlink to it and opening the symlink, or opening e.g.
>> > "./mem" after chdir'ing to "/proc/self".
>> >
>> > $ ln -s /proc/self self
>> > $ cat self/maps
>> > 60000000-602bc000 r-xp 00000000 fc:01 270375
>> /usr/bin/qemu-arm-static
>> > 604bc000-6050f000 rw-p 002bc000 fc:01 270375
>> /usr/bin/qemu-arm-static
>> > ...
>> >
>> > Signed-off-by: Zach Riggle <address@hidden>
>> > ---
>> > linux-user/syscall.c | 47 ++++++++++++++++++++++++++++--
>> -----------------
>> > 1 file changed, 28 insertions(+), 19 deletions(-)
>> >
>> > diff --git a/linux-user/syscall.c b/linux-user/syscall.c
>> > index 9bf901fa11..6c1f28a1f7 100644
>> > --- a/linux-user/syscall.c
>> > +++ b/linux-user/syscall.c
>> > @@ -7496,26 +7496,35 @@ static int open_self_auxv(void *cpu_env, int fd)
>> >
>> > static int is_proc_myself(const char *filename, const char *entry)
>> > {
>> > - if (!strncmp(filename, "/proc/", strlen("/proc/"))) {
>> > - filename += strlen("/proc/");
>> > - if (!strncmp(filename, "self/", strlen("self/"))) {
>> > - filename += strlen("self/");
>> > - } else if (*filename >= '1' && *filename <= '9') {
>> > - char myself[80];
>> > - snprintf(myself, sizeof(myself), "%d/", getpid());
>> > - if (!strncmp(filename, myself, strlen(myself))) {
>> > - filename += strlen(myself);
>> > - } else {
>> > - return 0;
>> > - }
>> > - } else {
>> > - return 0;
>> > - }
>> > - if (!strcmp(filename, entry)) {
>> > - return 1;
>> > - }
>> > + char proc_self_entry[PATH_MAX + 1];
>> > + char proc_self_entry_realpath[PATH_MAX + 1];
>> > + char filename_realpath[PATH_MAX + 1];
>> > +
>> > + if (PATH_MAX < snprintf(proc_self_entry,
>> > + sizeof(proc_self_entry),
>> > + "/proc/self/%s",
>> > + entry)) {
>> > + /* Full path to "entry" is too long to fit in the buffer */
>> > + return 0;
>> > }
>> > - return 0;
>> > +
>> > + if (!realpath(filename, filename_realpath)) {
>> > + /* File does not exist, or can't be canonicalized */
>> > + return 0;
>> > + }
>> > +
>> > + if (!realpath(proc_self_entry, proc_self_entry_realpath)) {
>> > + /* Procfs entry does not exist */
>> > + return 0;
>> > + }
>> > +
>> > + if (strcmp(filename_realpath, proc_self_entry_realpath) != 0) {
>> > + /* Paths are different */
>>
>> I think it doesn't work with /proc/map/exe (or other soft link in
>> /proc/self) as realpath will give the path of the executable and not the
>> path inside /proc so it will be true for any process with the same
>> executable (which in our case is qemu for all).
>>
>> Thanks,
>> Laurent
>>
>>
>