Re: [PATCH v2 1/7] binfmt: don't use MAP_DENYWRITE when loading shared libraries via uselib()

Guenter Roeck
2021-09-05T15:32:36Z

On Mon, Aug 16, 2021 at 09:48:34PM +0200, David Hildenbrand wrote:
> uselib() is the legacy systemcall for loading shared libraries.
> Nowadays, applications use dlopen() to load shared libraries, completely
> implemented in user space via mmap().
>
> For example, glibc uses MAP_COPY to mmap shared libraries. While this
> maps to MAP_PRIVATE | MAP_DENYWRITE on Linux, Linux ignores any
> MAP_DENYWRITE specification from user space in mmap.
>
> With this change, all remaining in-tree users of MAP_DENYWRITE use it
> to map an executable. We will be able to open shared libraries loaded
> via uselib() writable, just as we already can via dlopen() from user
> space.
>
> This is one step into the direction of removing MAP_DENYWRITE from the
> kernel. This can be considered a minor user space visible change.
>
> Acked-by: "Eric W. Biederman" <ebiederm@xmission.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>
> ---
> arch/x86/ia32/ia32_aout.c | 2 +-
> fs/binfmt_aout.c | 2 +-
> fs/binfmt_elf.c | 2 +-
> 3 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/arch/x86/ia32/ia32_aout.c b/arch/x86/ia32/ia32_aout.c
> index 5e5b9fc2747f..321d7b22ad2d 100644
> --- a/arch/x86/ia32/ia32_aout.c
> +++ b/arch/x86/ia32/ia32_aout.c
> @@ -293,7 +293,7 @@ static int load_aout_library(struct file *file)
> /* Now use mmap to map the library into memory. */
> error = vm_mmap(file, start_addr, ex.a_text + ex.a_data,
> PROT_READ | PROT_WRITE | PROT_EXEC,
> - MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE | MAP_32BIT,
> + MAP_FIXED | MAP_PRIVATE | MAP_32BIT,
> N_TXTOFF(ex));
> retval = error;
> if (error != start_addr)
> diff --git a/fs/binfmt_aout.c b/fs/binfmt_aout.c
> index 145917f734fe..d29de971d3f3 100644
> --- a/fs/binfmt_aout.c
> +++ b/fs/binfmt_aout.c
> @@ -309,7 +309,7 @@ static int load_aout_library(struct file *file)
> /* Now use mmap to map the library into memory. */
> error = vm_mmap(file, start_addr, ex.a_text + ex.a_data,
> PROT_READ | PROT_WRITE | PROT_EXEC,
> - MAP_FIXED | MAP_PRIVATE | MAP_DENYWRITE,
> + MAP_FIXED | MAP_PRIVATE;
> N_TXTOFF(ex));

Guess someone didn't care compile testing their code. This is now in
mainline.

Guenter