All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Vincent Fazio <vfazio@gmail.com>
Cc: Laurent Vivier <laurent@vivier.eu>,
	Vincent Fazio <vfazio@xes-inc.com>,
	qemu-trivial@nongnu.org, qemu-devel@nongnu.org
Subject: Re: [PATCH] linux-user/elfload: do not assume MAP_FIXED_NOREPLACE kernel support
Date: Mon, 15 Feb 2021 09:52:26 +0000	[thread overview]
Message-ID: <87o8glveme.fsf@linaro.org> (raw)
In-Reply-To: <CAOrEah7X3H7g7gSKFf-jD0nQ7YqnE+hUP7eq7Ozk8HfwYaxuqA@mail.gmail.com>


Vincent Fazio <vfazio@gmail.com> writes:

> On Sun, Feb 14, 2021 at 6:50 AM Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> Le 14/02/2021 à 12:24, Alex Bennée a écrit :
>> >
>> > Vincent Fazio <vfazio@xes-inc.com> writes:
>> >
>> >> From: Vincent Fazio <vfazio@gmail.com>
>> >>
>> >> Previously, pgd_find_hole_fallback assumed that if the build host's libc
>> >> had MAP_FIXED_NOREPLACE defined that the address returned by mmap would
>> >> match the requested address. This is not a safe assumption for Linux
>> >> kernels prior to 4.17
>> >
>> > It doesn't as we have in osdep.h:
>> >
>> >   #ifndef MAP_FIXED_NOREPLACE
>> >   #define MAP_FIXED_NOREPLACE 0
>> >   #endif
>> >
>> > which is to say to assume if MAP_FIXED_NOREPLACE is defined the kernel
>> > should have given us what we want otherwise we do the check.
>>
>>
>> But what is the purpose of the "if (MAP_FIXED_NOREPLACE != 0 ||"?
>> Can't we rely only on "mmap_start == (void *) align_start"?
>>
>> Thanks,
>> Laurent
>>
>
> I think we have to rely on address matching. The problem is
> specifically when MAP_FIXED_NOREPLACE is defined and is passed to mmap
> but the running kernel doesn't know what to do with the flag so
> returns a value that is not what was hinted at. Previously the code
> assumed that if MAP_FIXED_NOREPLACE was defined that the returned
> address would match, but that isn't always the case if the kernel
> doesn't have support for the flag. The 4.4, 4.9 and 4.14 LTS kernels
> are still in use and could run into this problem.

Ahh right so I think this is a case of binaries being built on a
different platform than kernel they are running on. In which case the
flag would be defined but the underlying kernel fails to identify it. Is
this a container like case by any chance?

If I'd read the man page closer:

   Note   that   older   kernels   which   do   not  recognize  the
   MAP_FIXED_NOREPLACE flag will typically (upon detecting a colli‐
   sion  with a preexisting mapping) fall back to a "non-MAP_FIXED"
   type of behavior: they will return an address that is  different
   from  the  requested  address.   Therefore,  backward-compatible
   software should check the returned address against the requested
   address.

so yes we should avoid short circuiting the return address check.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée


WARNING: multiple messages have this Message-ID (diff)
From: "Alex Bennée" <alex.bennee@linaro.org>
To: Vincent Fazio <vfazio@gmail.com>
Cc: qemu-trivial@nongnu.org, qemu-devel@nongnu.org,
	Laurent Vivier <laurent@vivier.eu>,
	Vincent Fazio <vfazio@xes-inc.com>
Subject: Re: [PATCH] linux-user/elfload: do not assume MAP_FIXED_NOREPLACE kernel support
Date: Mon, 15 Feb 2021 09:52:26 +0000	[thread overview]
Message-ID: <87o8glveme.fsf@linaro.org> (raw)
In-Reply-To: <CAOrEah7X3H7g7gSKFf-jD0nQ7YqnE+hUP7eq7Ozk8HfwYaxuqA@mail.gmail.com>


Vincent Fazio <vfazio@gmail.com> writes:

> On Sun, Feb 14, 2021 at 6:50 AM Laurent Vivier <laurent@vivier.eu> wrote:
>>
>> Le 14/02/2021 à 12:24, Alex Bennée a écrit :
>> >
>> > Vincent Fazio <vfazio@xes-inc.com> writes:
>> >
>> >> From: Vincent Fazio <vfazio@gmail.com>
>> >>
>> >> Previously, pgd_find_hole_fallback assumed that if the build host's libc
>> >> had MAP_FIXED_NOREPLACE defined that the address returned by mmap would
>> >> match the requested address. This is not a safe assumption for Linux
>> >> kernels prior to 4.17
>> >
>> > It doesn't as we have in osdep.h:
>> >
>> >   #ifndef MAP_FIXED_NOREPLACE
>> >   #define MAP_FIXED_NOREPLACE 0
>> >   #endif
>> >
>> > which is to say to assume if MAP_FIXED_NOREPLACE is defined the kernel
>> > should have given us what we want otherwise we do the check.
>>
>>
>> But what is the purpose of the "if (MAP_FIXED_NOREPLACE != 0 ||"?
>> Can't we rely only on "mmap_start == (void *) align_start"?
>>
>> Thanks,
>> Laurent
>>
>
> I think we have to rely on address matching. The problem is
> specifically when MAP_FIXED_NOREPLACE is defined and is passed to mmap
> but the running kernel doesn't know what to do with the flag so
> returns a value that is not what was hinted at. Previously the code
> assumed that if MAP_FIXED_NOREPLACE was defined that the returned
> address would match, but that isn't always the case if the kernel
> doesn't have support for the flag. The 4.4, 4.9 and 4.14 LTS kernels
> are still in use and could run into this problem.

Ahh right so I think this is a case of binaries being built on a
different platform than kernel they are running on. In which case the
flag would be defined but the underlying kernel fails to identify it. Is
this a container like case by any chance?

If I'd read the man page closer:

   Note   that   older   kernels   which   do   not  recognize  the
   MAP_FIXED_NOREPLACE flag will typically (upon detecting a colli‐
   sion  with a preexisting mapping) fall back to a "non-MAP_FIXED"
   type of behavior: they will return an address that is  different
   from  the  requested  address.   Therefore,  backward-compatible
   software should check the returned address against the requested
   address.

so yes we should avoid short circuiting the return address check.

Reviewed-by: Alex Bennée <alex.bennee@linaro.org>

-- 
Alex Bennée


  reply	other threads:[~2021-02-15  9:56 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-31  6:19 [PATCH] linux-user/elfload: do not assume MAP_FIXED_NOREPLACE kernel support Vincent Fazio
2021-01-31  6:19 ` Vincent Fazio
2021-02-13 21:44 ` Laurent Vivier
2021-02-13 21:44   ` Laurent Vivier
2021-02-14 11:24 ` Alex Bennée
2021-02-14 12:50   ` Laurent Vivier
2021-02-14 12:50     ` Laurent Vivier
2021-02-14 14:20     ` Vincent Fazio
2021-02-14 14:20       ` Vincent Fazio
2021-02-15  9:52       ` Alex Bennée [this message]
2021-02-15  9:52         ` Alex Bennée
2021-02-25 14:26         ` Vincent Fazio
2021-02-25 14:26           ` Vincent Fazio
2021-03-09 20:36 ` Laurent Vivier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=87o8glveme.fsf@linaro.org \
    --to=alex.bennee@linaro.org \
    --cc=laurent@vivier.eu \
    --cc=qemu-devel@nongnu.org \
    --cc=qemu-trivial@nongnu.org \
    --cc=vfazio@gmail.com \
    --cc=vfazio@xes-inc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.