linux-kernel - Re: [PATCH 3/4] mm: cma: Ensure that reservations never cross the low/high mem boundary

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]

Date:	Fri, 24 Oct 2014 11:53:25 +0900
From:	Joonsoo Kim <iamjoonsoo.kim@....com>
To:	Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>
Cc:	linux-mm@...ck.org, linux-kernel@...r.kernel.org,
	linux-arm-kernel@...ts.infradead.org, linux-sh@...r.kernel.org,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	Russell King - ARM Linux <linux@....linux.org.uk>,
	Michal Nazarewicz <mina86@...a86.com>
Subject: Re: [PATCH 3/4] mm: cma: Ensure that reservations never cross the
 low/high mem boundary

On Thu, Oct 23, 2014 at 05:33:47PM +0300, Laurent Pinchart wrote:
> Commit 95b0e655f914 ("ARM: mm: don't limit default CMA region only to
> low memory") extended CMA memory reservation to allow usage of high
> memory. It relied on commit f7426b983a6a ("mm: cma: adjust address limit
> to avoid hitting low/high memory boundary") to ensure that the reserved
> block never crossed the low/high memory boundary. While the
> implementation correctly lowered the limit, it failed to consider the
> case where the base..limit range crossed the low/high memory boundary
> with enough space on each side to reserve the requested size on either
> low or high memory.
> 
> Rework the base and limit adjustment to fix the problem. The function
> now starts by rejecting the reservation altogether for fixed
> reservations that cross the boundary, then adjust the limit if
> reservation from high memory is impossible, and finally first try to
> reserve from high memory first and then falls back to low memory.
> 
> Signed-off-by: Laurent Pinchart <laurent.pinchart+renesas@...asonboard.com>
> ---
>  mm/cma.c | 58 ++++++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 44 insertions(+), 14 deletions(-)
> 
> diff --git a/mm/cma.c b/mm/cma.c
> index 6b14346..b83597b 100644
> --- a/mm/cma.c
> +++ b/mm/cma.c
> @@ -247,23 +247,38 @@ int __init cma_declare_contiguous(phys_addr_t base,
>  		return -EINVAL;
>  
>  	/*
> -	 * adjust limit to avoid crossing low/high memory boundary for
> +	 * Adjust limit and base to avoid crossing low/high memory boundary for
>  	 * automatically allocated regions
>  	 */
> -	if (((limit == 0 || limit > memblock_end) &&
> -	     (memblock_end - size < highmem_start &&
> -	      memblock_end > highmem_start)) ||
> -	    (!fixed && limit > highmem_start && limit - size < highmem_start)) {
> -		limit = highmem_start;
> -	}
>  
> -	if (fixed && base < highmem_start && base+size > highmem_start) {
> +	/*
> +	 * If allocating at a fixed base the request region must not cross the
> +	 * low/high memory boundary.
> +	 */
> +	if (fixed && base < highmem_start && base + size > highmem_start) {
>  		ret = -EINVAL;
>  		pr_err("Region at %08lx defined on low/high memory boundary (%08lx)\n",
>  			(unsigned long)base, (unsigned long)highmem_start);
>  		goto err;
>  	}
>  
> +	/*
> +	 * If the limit is unspecified or above the memblock end, its effective
> +	 * value will be the memblock end. Set it explicitly to simplify further
> +	 * checks.
> +	 */
> +	if (limit == 0 || limit > memblock_end)
> +		limit = memblock_end;
> +
> +	/*
> +	 * If the limit is above the highmem start by less than the reserved
> +	 * size allocation in highmem won't be possible. Lower the limit to the
> +	 * lowmem end.
> +	 */
> +	if (limit > highmem_start && limit - size < highmem_start)
> +		limit = highmem_start;
> +
> +

How about removing this check?
Without this check, memblock_alloc_range would be failed and we can
go fallback correctly. So, this is redundant, IMO.

Thanks.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/