linux-kernel - Re: [RFC PATCH 1/5] mm: mremap: Fix new_addr being used as a hint with MREMAP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <CADMPHGtENiESdnHsoMCsVA2FNBg4jLS4kMZJfGOmvv7=9k_+HA@mail.gmail.com>
Date: Wed, 11 Dec 2024 13:46:17 -0300
From: Marco Vanotti <mvanotti@...gle.com>
To: Brian Geffon <bgeffon@...gle.com>
Cc: Andrew Morton <akpm@...ux-foundation.org>, Lorenzo Stoakes <lorenzo.stoakes@...cle.com>, 
	Jann Horn <jannh@...gle.com>, Vlastimil Babka <vbabka@...e.cz>, 
	"Liam R. Howlett" <Liam.Howlett@...cle.com>, linux-mm@...ck.org, linux-kernel@...r.kernel.org
Subject: Re: [RFC PATCH 1/5] mm: mremap: Fix new_addr being used as a hint
 with MREMAP_DONTUNMAP

On Tue, Dec 10, 2024 at 6:31 PM Brian Geffon <bgeffon@...gle.com> wrote:
>
> Two non-mutually exclusive paths can land in mremap_to, MREMAP_FIXED
> and MREMAP_DONTUNMAP which are called from mremap(). In the case of
> MREMAP_FIXED we must validate the new_addr to ensure that the new
> address is valid. In the case of MREMAP_DONTUNMAP without MREMAP_FIXED
> a new address is specified as a hint, just like it would be in the
> case of mmap. In this second case we don't need to perform any checks
> because get_unmapped_area() will align new_addr, just like it would in
> the case of mmap.
>
> This patch only fixes the behavior that was inadvertently added
> with MREMAP_DONTUNMAP.
>
> v2:
>   - Addressed comment from Marco Vanotti to consolidate these checks
>     into existing MREMAP_FIXED blocks.
>
> Signed-off-by: Brian Geffon <bgeffon@...gle.com>
> Reported-by: Marco Vanotti <mvanotti@...gle.com>
Reviewed-By: Marco Vanotti <mvanotti@...gle.com>
> ---
>  mm/mremap.c | 29 +++++++++++++++++++----------
>  1 file changed, 19 insertions(+), 10 deletions(-)
>
> diff --git a/mm/mremap.c b/mm/mremap.c
> index 60473413836b..62aec72bbe42 100644
> --- a/mm/mremap.c
> +++ b/mm/mremap.c
> @@ -912,16 +912,6 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
>         unsigned long ret;
>         unsigned long map_flags = 0;
>
> -       if (offset_in_page(new_addr))
> -               return -EINVAL;
> -
> -       if (new_len > TASK_SIZE || new_addr > TASK_SIZE - new_len)
> -               return -EINVAL;
> -
> -       /* Ensure the old/new locations do not overlap */
> -       if (addr + old_len > new_addr && new_addr + new_len > addr)
> -               return -EINVAL;
> -
>         /*
>          * move_vma() need us to stay 4 maps below the threshold, otherwise
>          * it will bail out at the very beginning.
> @@ -940,6 +930,25 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
>                 return -ENOMEM;
>
>         if (flags & MREMAP_FIXED) {
> +               /*
> +                * Two non-mutually exclusive paths can land in mremap_to, MREMAP_FIXED
> +                * and MREMAP_DONTUNMAP which are called from mremap(). In the case of
> +                * MREMAP_FIXED we must validate the new_addr to ensure that the new
> +                * address is valid. In the case of MREMAP_DONTUNMAP without MREMAP_FIXED
> +                * a new address is specified as a hint, just like it would be in the
> +                * case of mmap. In this second case we don't need to perform any checks
> +                * because get_unmapped_area() will align new_addr, just like it would in
> +                * the case of mmap.
> +                */
> +               if (offset_in_page(new_addr))
> +                       return -EINVAL;
> +
> +               if (new_len > TASK_SIZE || new_addr > TASK_SIZE - new_len)
> +                       return -EINVAL;
> +
> +               /* Ensure the old/new locations do not overlap */
> +               if (addr + old_len > new_addr && new_addr + new_len > addr)
> +                       return -EINVAL;
>                 /*
>                  * In mremap_to().
>                  * VMA is moved to dst address, and munmap dst first.
> --
> 2.47.0.338.g60cca15819-goog
>

Download attachment "smime.p7s" of type "application/pkcs7-signature" (5294 bytes)