linux-kernel - Re: [RFC][PATCH] CMA: drivers/base/Kconfig: restrict CMA size to non-zero value

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:	Mon, 19 May 2014 10:47:12 +0900
From:	Gioh Kim <gioh.kim@....com>
To:	Michal Nazarewicz <mina86@...a86.com>,
	Joonsoo Kim <iamjoonsoo.kim@....com>,
	Minchan Kim <minchan.kim@....com>
CC:	Andrew Morton <akpm@...ux-foundation.org>,
	Rik van Riel <riel@...hat.com>,
	Laura Abbott <lauraa@...eaurora.org>, linux-mm@...ck.org,
	linux-kernel@...r.kernel.org,
	Heesub Shin <heesub.shin@...sung.com>,
	Mel Gorman <mgorman@...e.de>,
	Johannes Weiner <hannes@...xchg.org>,
	Marek Szyprowski <m.szyprowski@...sung.com>,
	이건호 <gunho.lee@....com>, gurugio@...il.com
Subject: Re: [RFC][PATCH] CMA: drivers/base/Kconfig: restrict CMA size to
 non-zero value

Thank you for your advice. I didn't notice it.

I'm adding followings according to your advice:

- range restrict for CMA_SIZE_MBYTES and *CMA_SIZE_PERCENTAGE*
I think this can prevent the wrong kernel option.

- change size_cmdline into default value SZ_16M
I am not sure this can prevent if cma=0 cmdline option is also with base and limit options.


I don't know how to send the second patch.
Please pardon me that I just copy the patch here.

--------------------------------- 8< -------------------------------------
 From c283eaac41b044a2abb11cfd32a60fff034633c3 Mon Sep 17 00:00:00 2001
From: Gioh Kim <gioh.kim@....com>
Date: Fri, 16 May 2014 16:15:43 +0900
Subject: [PATCH] drivers/base/Kconfig: restrict CMA size to non-zero value

The size of CMA area must be larger than zero.
If the size is zero, all physically-contiguous allocation
can be failed.

Signed-off-by: Gioh Kim <gioh.kim@....co.kr>
---
  drivers/base/Kconfig          |   14 ++++++++++++--
  drivers/base/dma-contiguous.c |    3 ++-
  2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig
index 4b7b452..a7292ac 100644
--- a/drivers/base/Kconfig
+++ b/drivers/base/Kconfig
@@ -222,17 +222,27 @@ config DMA_CMA
  if  DMA_CMA
  comment "Default contiguous memory area size:"

+config CMA_SIZE_MBYTES_DEFAULT
+       int
+       default 16
+
+config CMA_SIZE_MBYTES_MAX
+       int
+       default 1024
+
  config CMA_SIZE_MBYTES
         int "Size in Mega Bytes"
         depends on !CMA_SIZE_SEL_PERCENTAGE
-       default 16
+       range 1 CMA_SIZE_MBYTES_MAX
+       default CMA_SIZE_MBYTES_DEFAULT
         help
           Defines the size (in MiB) of the default memory area for Contiguous
-         Memory Allocator.
+         Memory Allocator. This value must be larger than zero.

  config CMA_SIZE_PERCENTAGE
         int "Percentage of total memory"
         depends on !CMA_SIZE_SEL_MBYTES
+       range 1 100
         default 10
         help
           Defines the size of the default memory area for Contiguous Memory
diff --git a/drivers/base/dma-contiguous.c b/drivers/base/dma-contiguous.c
index b056661..5b70442 100644
--- a/drivers/base/dma-contiguous.c
+++ b/drivers/base/dma-contiguous.c
@@ -125,7 +125,8 @@ void __init dma_contiguous_reserve(phys_addr_t limit)
         pr_debug("%s(limit %08lx)\n", __func__, (unsigned long)limit);

         if (size_cmdline != -1) {
-               selected_size = size_cmdline;
+               selected_size = ((size_cmdline == 0) ?
+                                CONFIG_CMA_SIZE_MBYTES_DEFAULT : size_cmdline);
                 selected_base = base_cmdline;
                 selected_limit = min_not_zero(limit_cmdline, limit);
                 if (base_cmdline + size_cmdline == limit_cmdline)
--
1.7.9.5


2014-05-17 오전 2:45, Michal Nazarewicz 쓴 글:
> On Fri, May 16 2014, Gioh Kim wrote:
>> If CMA_SIZE_MBYTES is allowed to be zero, there should be defense code
>> to check CMA is initlaized correctly. And atomic_pool initialization
>> should be done by __alloc_remap_buffer instead of
>> __alloc_from_contiguous if __alloc_from_contiguous is failed.
>
> Agreed, and this is the correct fix.
>
>> IMPO, it is more simple and powerful to restrict CMA_SIZE_MBYTES_MAX
>> configuration to be larger than zero.
>
> No, because it makes it impossible to have CMA disabled by default and
> only enabled if command line argument is given.
>
> Furthermore, your patch does *not* guarantee CMA region to always be
> allocated.  If CMA_SIZE_SEL_PERCENTAGE is selected for instance.  Or if
> user explicitly passes 0 on command line.
>
>
>
--
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/