qemu-trivial
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Qemu-trivial] [Qemu-devel] [PATCH v2] linux-user: fix is_proc_mysel


From: Zach Riggle
Subject: Re: [Qemu-trivial] [Qemu-devel] [PATCH v2] linux-user: fix is_proc_myself to check the paths via realpath
Date: Fri, 10 Nov 2017 19:47:00 -0600

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



reply via email to

[Prev in Thread] Current Thread [Next in Thread]