linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm: add ___GFP_NOINIT flag which disables zeroing on alloc
       [not found] <CGME20210329054156epcas2p31650fa755e6cbcc55c4f33a79878256f@epcas2p3.samsung.com>
@ 2021-03-29  5:29 ` Hyunsoon Kim
  2021-03-29  6:34   ` Leon Romanovsky
                     ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Hyunsoon Kim @ 2021-03-29  5:29 UTC (permalink / raw)
  To: Andrew Morton; +Cc: dseok.yi, h10.kim, linux-mm, linux-kernel

This patch allows programmer to avoid zero initialization on page
allocation even when the kernel config "CONFIG_INIT_ON_ALLOC_DEFAULT"
is enabled. The configuration is made to prevent uninitialized
heap memory flaws, and Android has applied this for security and
deterministic execution times. Please refer to below.

https://android-review.googlesource.com/c/kernel/common/+/1235132

However, there is a case that the zeroing page memory is unnecessary
when the page is used on specific purpose and will be zeroed
automatically by hardware that accesses the memory through DMA.
For instance, page allocation used for IP packet reception from Exynos
modem is solely used for packet reception. Although the page will be
freed eventually and reused for some other purpose, initialization at
that moment of reuse will be sufficient to avoid uninitialized heap
memory flaws. To support this kind of control, this patch creates new
gfp type called ___GFP_NOINIT, that allows no zeroing at the moment
of page allocation, called by many related APIs such as page_frag_alloc,
alloc_pages, etc.

Signed-off-by: Hyunsoon Kim <h10.kim@samsung.com>
---
 include/linux/gfp.h | 2 ++
 include/linux/mm.h  | 4 +++-
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/include/linux/gfp.h b/include/linux/gfp.h
index 8572a14..4ddd947 100644
--- a/include/linux/gfp.h
+++ b/include/linux/gfp.h
@@ -58,6 +58,8 @@ struct vm_area_struct;
 #else
 #define ___GFP_NOLOCKDEP	0
 #endif
+#define ___GFP_NOINIT		0x1000000u
+
 /* If the above are modified, __GFP_BITS_SHIFT may need updating */
 
 /*
diff --git a/include/linux/mm.h b/include/linux/mm.h
index 8ba4342..06a23bb 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2907,7 +2907,9 @@ static inline void kernel_unpoison_pages(struct page *page, int numpages) { }
 DECLARE_STATIC_KEY_FALSE(init_on_alloc);
 static inline bool want_init_on_alloc(gfp_t flags)
 {
-	if (static_branch_unlikely(&init_on_alloc))
+	if (flags & ___GFP_NOINIT)
+		return false;
+	else if (static_branch_unlikely(&init_on_alloc))
 		return true;
 	return flags & __GFP_ZERO;
 }
-- 
2.7.4



^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm: add ___GFP_NOINIT flag which disables zeroing on alloc
  2021-03-29  5:29 ` [PATCH] mm: add ___GFP_NOINIT flag which disables zeroing on alloc Hyunsoon Kim
@ 2021-03-29  6:34   ` Leon Romanovsky
  2021-03-29 10:12     ` Hyunsoon Kim
  2021-03-29  9:47   ` kernel test robot
  2021-03-29 10:53   ` David Hildenbrand
  2 siblings, 1 reply; 8+ messages in thread
From: Leon Romanovsky @ 2021-03-29  6:34 UTC (permalink / raw)
  To: Hyunsoon Kim; +Cc: Andrew Morton, dseok.yi, linux-mm, linux-kernel

On Mon, Mar 29, 2021 at 02:29:10PM +0900, Hyunsoon Kim wrote:
> This patch allows programmer to avoid zero initialization on page
> allocation even when the kernel config "CONFIG_INIT_ON_ALLOC_DEFAULT"
> is enabled. The configuration is made to prevent uninitialized
> heap memory flaws, and Android has applied this for security and
> deterministic execution times. Please refer to below.
> 
> https://android-review.googlesource.com/c/kernel/common/+/1235132
> 
> However, there is a case that the zeroing page memory is unnecessary
> when the page is used on specific purpose and will be zeroed
> automatically by hardware that accesses the memory through DMA.
> For instance, page allocation used for IP packet reception from Exynos
> modem is solely used for packet reception. Although the page will be
> freed eventually and reused for some other purpose, initialization at
> that moment of reuse will be sufficient to avoid uninitialized heap
> memory flaws. To support this kind of control, this patch creates new
> gfp type called ___GFP_NOINIT, that allows no zeroing at the moment
> of page allocation, called by many related APIs such as page_frag_alloc,
> alloc_pages, etc.
> 
> Signed-off-by: Hyunsoon Kim <h10.kim@samsung.com>
> ---
>  include/linux/gfp.h | 2 ++
>  include/linux/mm.h  | 4 +++-
>  2 files changed, 5 insertions(+), 1 deletion(-)

Let's assume that we will use this new flag, and users are smart enough
to figure when it needs to be used, what will be the performance gain?

Thanks


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm: add ___GFP_NOINIT flag which disables zeroing on alloc
  2021-03-29  5:29 ` [PATCH] mm: add ___GFP_NOINIT flag which disables zeroing on alloc Hyunsoon Kim
  2021-03-29  6:34   ` Leon Romanovsky
@ 2021-03-29  9:47   ` kernel test robot
  2021-03-29 10:53   ` David Hildenbrand
  2 siblings, 0 replies; 8+ messages in thread
From: kernel test robot @ 2021-03-29  9:47 UTC (permalink / raw)
  To: Hyunsoon Kim, Andrew Morton
  Cc: kbuild-all, Linux Memory Management List, dseok.yi, h10.kim,
	linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3983 bytes --]

Hi Hyunsoon,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on v5.12-rc5]
[also build test WARNING on next-20210326]
[cannot apply to hnaz-linux-mm/master]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Hyunsoon-Kim/mm-add-___GFP_NOINIT-flag-which-disables-zeroing-on-alloc/20210329-134447
base:    a5e13c6df0e41702d2b2c77c8ad41677ebb065b3
config: m68k-randconfig-s032-20210329 (attached as .config)
compiler: m68k-linux-gcc (GCC) 9.3.0
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # apt-get install sparse
        # sparse version: v0.6.3-277-gc089cd2d-dirty
        # https://github.com/0day-ci/linux/commit/e952189333dd008d07e0b80235f6e04a01c1e052
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Hyunsoon-Kim/mm-add-___GFP_NOINIT-flag-which-disables-zeroing-on-alloc/20210329-134447
        git checkout e952189333dd008d07e0b80235f6e04a01c1e052
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross C=1 CF='-fdiagnostic-prefix -D__CHECK_ENDIAN__' ARCH=m68k 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>


sparse warnings: (new ones prefixed by >>)
   net/core/sock.c: note: in included file (through include/linux/bvec.h, include/linux/skbuff.h, include/linux/ip.h, include/net/ip.h, ...):
>> include/linux/mm.h:2910:13: sparse: sparse: restricted gfp_t degrades to integer
   net/core/sock.c:2011:9: sparse: sparse: context imbalance in 'sk_clone_lock' - wrong count at exit
   net/core/sock.c:2015:6: sparse: sparse: context imbalance in 'sk_free_unlock_clone' - unexpected unlock
   net/core/sock.c:3593:13: sparse: sparse: context imbalance in 'proto_seq_start' - wrong count at exit
   net/core/sock.c:3605:13: sparse: sparse: context imbalance in 'proto_seq_stop' - wrong count at exit
--
   init/main.c:291:16: sparse: sparse: cast to restricted __le32
   init/main.c:291:16: sparse: sparse: cast to restricted __le32
   init/main.c:291:16: sparse: sparse: cast to restricted __le32
   init/main.c:291:16: sparse: sparse: cast to restricted __le32
   init/main.c:291:16: sparse: sparse: cast to restricted __le32
   init/main.c:291:16: sparse: sparse: cast to restricted __le32
   init/main.c:292:16: sparse: sparse: cast to restricted __le32
   init/main.c:292:16: sparse: sparse: cast to restricted __le32
   init/main.c:292:16: sparse: sparse: cast to restricted __le32
   init/main.c:292:16: sparse: sparse: cast to restricted __le32
   init/main.c:292:16: sparse: sparse: cast to restricted __le32
   init/main.c:292:16: sparse: sparse: cast to restricted __le32
   init/main.c: note: in included file (through include/linux/ring_buffer.h, include/linux/trace_events.h, include/trace/syscall.h, ...):
>> include/linux/mm.h:2910:13: sparse: sparse: restricted gfp_t degrades to integer
--
   mm/page_alloc.c: note: in included file:
>> include/linux/mm.h:2910:13: sparse: sparse: restricted gfp_t degrades to integer
   mm/page_alloc.c: note: in included file (through include/linux/mm.h):
   include/linux/gfp.h:343:27: sparse: sparse: restricted gfp_t degrades to integer

vim +2910 include/linux/mm.h

  2906	
  2907	DECLARE_STATIC_KEY_FALSE(init_on_alloc);
  2908	static inline bool want_init_on_alloc(gfp_t flags)
  2909	{
> 2910		if (flags & ___GFP_NOINIT)
  2911			return false;
  2912		else if (static_branch_unlikely(&init_on_alloc))
  2913			return true;
  2914		return flags & __GFP_ZERO;
  2915	}
  2916	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org

[-- Attachment #2: .config.gz --]
[-- Type: application/gzip, Size: 24847 bytes --]

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm: add ___GFP_NOINIT flag which disables zeroing on alloc
  2021-03-29  6:34   ` Leon Romanovsky
@ 2021-03-29 10:12     ` Hyunsoon Kim
  2021-03-31  3:55       ` Leon Romanovsky
  0 siblings, 1 reply; 8+ messages in thread
From: Hyunsoon Kim @ 2021-03-29 10:12 UTC (permalink / raw)
  To: Leon Romanovsky; +Cc: Andrew Morton, dseok.yi, linux-mm, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 2144 bytes --]

On Mon, Mar 29, 2021 at 09:34:31AM +0300, Leon Romanovsky wrote:
> On Mon, Mar 29, 2021 at 02:29:10PM +0900, Hyunsoon Kim wrote:
> > This patch allows programmer to avoid zero initialization on page
> > allocation even when the kernel config "CONFIG_INIT_ON_ALLOC_DEFAULT"
> > is enabled. The configuration is made to prevent uninitialized
> > heap memory flaws, and Android has applied this for security and
> > deterministic execution times. Please refer to below.
> > 
> > https://android-review.googlesource.com/c/kernel/common/+/1235132
> > 
> > However, there is a case that the zeroing page memory is unnecessary
> > when the page is used on specific purpose and will be zeroed
> > automatically by hardware that accesses the memory through DMA.
> > For instance, page allocation used for IP packet reception from Exynos
> > modem is solely used for packet reception. Although the page will be
> > freed eventually and reused for some other purpose, initialization at
> > that moment of reuse will be sufficient to avoid uninitialized heap
> > memory flaws. To support this kind of control, this patch creates new
> > gfp type called ___GFP_NOINIT, that allows no zeroing at the moment
> > of page allocation, called by many related APIs such as page_frag_alloc,
> > alloc_pages, etc.
> > 
> > Signed-off-by: Hyunsoon Kim <h10.kim@samsung.com>
> > ---
> >  include/linux/gfp.h | 2 ++
> >  include/linux/mm.h  | 4 +++-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> 
> Let's assume that we will use this new flag, and users are smart enough
> to figure when it needs to be used, what will be the performance gain?
> 
> Thanks

For instance, there are four memory access (either read or write) done
by the system; memory write due to page allocation for reserving memory
for modem hardware, memory write on the page by modem hardware,
read and write incurred by copy_to_user operation by iperf reading
the incoming network data. Theoretically, we can expect 1/4 of power
saving on DRAM bandwidth. By performing simple iperf test with download
UDP 800Mbps, we saw 5-6mA power gain by disabling
CONFIG_INIT_ON_ALLOC_DEFAULT.

Thanks

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm: add ___GFP_NOINIT flag which disables zeroing on alloc
  2021-03-29  5:29 ` [PATCH] mm: add ___GFP_NOINIT flag which disables zeroing on alloc Hyunsoon Kim
  2021-03-29  6:34   ` Leon Romanovsky
  2021-03-29  9:47   ` kernel test robot
@ 2021-03-29 10:53   ` David Hildenbrand
  2021-03-30  1:44     ` Hyunsoon Kim
  2 siblings, 1 reply; 8+ messages in thread
From: David Hildenbrand @ 2021-03-29 10:53 UTC (permalink / raw)
  To: Hyunsoon Kim, Andrew Morton; +Cc: dseok.yi, linux-mm, linux-kernel

On 29.03.21 07:29, Hyunsoon Kim wrote:
> This patch allows programmer to avoid zero initialization on page
> allocation even when the kernel config "CONFIG_INIT_ON_ALLOC_DEFAULT"
> is enabled. The configuration is made to prevent uninitialized
> heap memory flaws, and Android has applied this for security and
> deterministic execution times. Please refer to below.
> 
> https://android-review.googlesource.com/c/kernel/common/+/1235132
> 
> However, there is a case that the zeroing page memory is unnecessary
> when the page is used on specific purpose and will be zeroed
> automatically by hardware that accesses the memory through DMA.
> For instance, page allocation used for IP packet reception from Exynos
> modem is solely used for packet reception. Although the page will be
> freed eventually and reused for some other purpose, initialization at
> that moment of reuse will be sufficient to avoid uninitialized heap
> memory flaws. To support this kind of control, this patch creates new
> gfp type called ___GFP_NOINIT, that allows no zeroing at the moment
> of page allocation, called by many related APIs such as page_frag_alloc,
> alloc_pages, etc.
> 
> Signed-off-by: Hyunsoon Kim <h10.kim@samsung.com>
> ---
>   include/linux/gfp.h | 2 ++
>   include/linux/mm.h  | 4 +++-
>   2 files changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> index 8572a14..4ddd947 100644
> --- a/include/linux/gfp.h
> +++ b/include/linux/gfp.h
> @@ -58,6 +58,8 @@ struct vm_area_struct;
>   #else
>   #define ___GFP_NOLOCKDEP	0
>   #endif
> +#define ___GFP_NOINIT		0x1000000u
> +
>   /* If the above are modified, __GFP_BITS_SHIFT may need updating */
>   
>   /*
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 8ba4342..06a23bb 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2907,7 +2907,9 @@ static inline void kernel_unpoison_pages(struct page *page, int numpages) { }
>   DECLARE_STATIC_KEY_FALSE(init_on_alloc);
>   static inline bool want_init_on_alloc(gfp_t flags)
>   {
> -	if (static_branch_unlikely(&init_on_alloc))
> +	if (flags & ___GFP_NOINIT)
> +		return false;
> +	else if (static_branch_unlikely(&init_on_alloc))
>   		return true;
>   	return flags & __GFP_ZERO;
>   }
> 

We discussed that in the past - whatever leaves the buddy shall be 
initialized. This is a security feature, not something random kernel 
modules should be able to hack around.

We also discussed back then to allow other allocators to eventually be 
able to optimize in the future if we are sure it really makes sense. 
Then, however, we need a new API that is not available to random 
modules, instead of exposing ___GFP_NOINIT to anybody out there in the 
system.

Nacked-by: David Hildenbrand <david@redhat.com>

-- 
Thanks,

David / dhildenb



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm: add ___GFP_NOINIT flag which disables zeroing on alloc
  2021-03-29 10:53   ` David Hildenbrand
@ 2021-03-30  1:44     ` Hyunsoon Kim
  2021-03-30  8:52       ` David Hildenbrand
  0 siblings, 1 reply; 8+ messages in thread
From: Hyunsoon Kim @ 2021-03-30  1:44 UTC (permalink / raw)
  To: David Hildenbrand; +Cc: Andrew Morton, dseok.yi, linux-mm, linux-kernel

[-- Attachment #1: Type: text/plain, Size: 3779 bytes --]

On Mon, Mar 29, 2021 at 12:53:48PM +0200, David Hildenbrand wrote:
> On 29.03.21 07:29, Hyunsoon Kim wrote:
> >This patch allows programmer to avoid zero initialization on page
> >allocation even when the kernel config "CONFIG_INIT_ON_ALLOC_DEFAULT"
> >is enabled. The configuration is made to prevent uninitialized
> >heap memory flaws, and Android has applied this for security and
> >deterministic execution times. Please refer to below.
> >
> >https://android-review.googlesource.com/c/kernel/common/+/1235132
> >
> >However, there is a case that the zeroing page memory is unnecessary
> >when the page is used on specific purpose and will be zeroed
> >automatically by hardware that accesses the memory through DMA.
> >For instance, page allocation used for IP packet reception from Exynos
> >modem is solely used for packet reception. Although the page will be
> >freed eventually and reused for some other purpose, initialization at
> >that moment of reuse will be sufficient to avoid uninitialized heap
> >memory flaws. To support this kind of control, this patch creates new
> >gfp type called ___GFP_NOINIT, that allows no zeroing at the moment
> >of page allocation, called by many related APIs such as page_frag_alloc,
> >alloc_pages, etc.
> >
> >Signed-off-by: Hyunsoon Kim <h10.kim@samsung.com>
> >---
> >  include/linux/gfp.h | 2 ++
> >  include/linux/mm.h  | 4 +++-
> >  2 files changed, 5 insertions(+), 1 deletion(-)
> >
> >diff --git a/include/linux/gfp.h b/include/linux/gfp.h
> >index 8572a14..4ddd947 100644
> >--- a/include/linux/gfp.h
> >+++ b/include/linux/gfp.h
> >@@ -58,6 +58,8 @@ struct vm_area_struct;
> >  #else
> >  #define ___GFP_NOLOCKDEP	0
> >  #endif
> >+#define ___GFP_NOINIT		0x1000000u
> >+
> >  /* If the above are modified, __GFP_BITS_SHIFT may need updating */
> >  /*
> >diff --git a/include/linux/mm.h b/include/linux/mm.h
> >index 8ba4342..06a23bb 100644
> >--- a/include/linux/mm.h
> >+++ b/include/linux/mm.h
> >@@ -2907,7 +2907,9 @@ static inline void kernel_unpoison_pages(struct page *page, int numpages) { }
> >  DECLARE_STATIC_KEY_FALSE(init_on_alloc);
> >  static inline bool want_init_on_alloc(gfp_t flags)
> >  {
> >-	if (static_branch_unlikely(&init_on_alloc))
> >+	if (flags & ___GFP_NOINIT)
> >+		return false;
> >+	else if (static_branch_unlikely(&init_on_alloc))
> >  		return true;
> >  	return flags & __GFP_ZERO;
> >  }
> >
> 
> We discussed that in the past - whatever leaves the buddy shall be
> initialized. This is a security feature, not something random kernel modules
> should be able to hack around.
> 
> We also discussed back then to allow other allocators to eventually be able
> to optimize in the future if we are sure it really makes sense. Then,
> however, we need a new API that is not available to random modules, instead
> of exposing ___GFP_NOINIT to anybody out there in the system.
> 
> Nacked-by: David Hildenbrand <david@redhat.com>
> 
> -- 
> Thanks,
> 
> David / dhildenb

If you don't mind, may i ask you exactly what security flaws you are expecting
from uninitialized value allocation? I can think of below scenario:

1. Security related value is freed by security system.
2. Malicious module get allocation to the memory region that is freed by above.
3. Malicious module uses that uninitialized value, and breach the security.

Could you please confirm that I am think in the right way? If so, isn't it
possible to make the security system to zero on free? I am not talking about
CONFIG_INIT_ON_FREE_DEFAULT_ON. I am just suggesting that isn't it better to
make programs that generate important values to be forced to initialize on
free, instead of making whole system to zeroing on alloc always, resulting
in performance downgrade? I think this approach can make enhancement.

Thanks,

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm: add ___GFP_NOINIT flag which disables zeroing on alloc
  2021-03-30  1:44     ` Hyunsoon Kim
@ 2021-03-30  8:52       ` David Hildenbrand
  0 siblings, 0 replies; 8+ messages in thread
From: David Hildenbrand @ 2021-03-30  8:52 UTC (permalink / raw)
  To: Hyunsoon Kim; +Cc: Andrew Morton, dseok.yi, linux-mm, linux-kernel

On 30.03.21 03:44, Hyunsoon Kim wrote:
> On Mon, Mar 29, 2021 at 12:53:48PM +0200, David Hildenbrand wrote:
>> On 29.03.21 07:29, Hyunsoon Kim wrote:
>>> This patch allows programmer to avoid zero initialization on page
>>> allocation even when the kernel config "CONFIG_INIT_ON_ALLOC_DEFAULT"
>>> is enabled. The configuration is made to prevent uninitialized
>>> heap memory flaws, and Android has applied this for security and
>>> deterministic execution times. Please refer to below.
>>>
>>> https://android-review.googlesource.com/c/kernel/common/+/1235132
>>>
>>> However, there is a case that the zeroing page memory is unnecessary
>>> when the page is used on specific purpose and will be zeroed
>>> automatically by hardware that accesses the memory through DMA.
>>> For instance, page allocation used for IP packet reception from Exynos
>>> modem is solely used for packet reception. Although the page will be
>>> freed eventually and reused for some other purpose, initialization at
>>> that moment of reuse will be sufficient to avoid uninitialized heap
>>> memory flaws. To support this kind of control, this patch creates new
>>> gfp type called ___GFP_NOINIT, that allows no zeroing at the moment
>>> of page allocation, called by many related APIs such as page_frag_alloc,
>>> alloc_pages, etc.
>>>
>>> Signed-off-by: Hyunsoon Kim <h10.kim@samsung.com>
>>> ---
>>>   include/linux/gfp.h | 2 ++
>>>   include/linux/mm.h  | 4 +++-
>>>   2 files changed, 5 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/include/linux/gfp.h b/include/linux/gfp.h
>>> index 8572a14..4ddd947 100644
>>> --- a/include/linux/gfp.h
>>> +++ b/include/linux/gfp.h
>>> @@ -58,6 +58,8 @@ struct vm_area_struct;
>>>   #else
>>>   #define ___GFP_NOLOCKDEP	0
>>>   #endif
>>> +#define ___GFP_NOINIT		0x1000000u
>>> +
>>>   /* If the above are modified, __GFP_BITS_SHIFT may need updating */
>>>   /*
>>> diff --git a/include/linux/mm.h b/include/linux/mm.h
>>> index 8ba4342..06a23bb 100644
>>> --- a/include/linux/mm.h
>>> +++ b/include/linux/mm.h
>>> @@ -2907,7 +2907,9 @@ static inline void kernel_unpoison_pages(struct page *page, int numpages) { }
>>>   DECLARE_STATIC_KEY_FALSE(init_on_alloc);
>>>   static inline bool want_init_on_alloc(gfp_t flags)
>>>   {
>>> -	if (static_branch_unlikely(&init_on_alloc))
>>> +	if (flags & ___GFP_NOINIT)
>>> +		return false;
>>> +	else if (static_branch_unlikely(&init_on_alloc))
>>>   		return true;
>>>   	return flags & __GFP_ZERO;
>>>   }
>>>
>>
>> We discussed that in the past - whatever leaves the buddy shall be
>> initialized. This is a security feature, not something random kernel modules
>> should be able to hack around.
>>
>> We also discussed back then to allow other allocators to eventually be able
>> to optimize in the future if we are sure it really makes sense. Then,
>> however, we need a new API that is not available to random modules, instead
>> of exposing ___GFP_NOINIT to anybody out there in the system.
>>
>> Nacked-by: David Hildenbrand <david@redhat.com>
>>
>> -- 
>> Thanks,
>>
>> David / dhildenb
> 
> If you don't mind, may i ask you exactly what security flaws you are expecting
> from uninitialized value allocation? I can think of below scenario:
> 
> 1. Security related value is freed by security system.
> 2. Malicious module get allocation to the memory region that is freed by above.
> 3. Malicious module uses that uninitialized value, and breach the security.
> 

I think one of the most important cases are "Content of process A is 
leaked via driver/mechanism X to process B". Or "Kernel content is 
leaked via driver/mechanism X to process Y".

> Could you please confirm that I am think in the right way? If so, isn't it
> possible to make the security system to zero on free? I am not talking about
> CONFIG_INIT_ON_FREE_DEFAULT_ON. I am just suggesting that isn't it better to
> make programs that generate important values to be forced to initialize on
> free, instead of making whole system to zeroing on alloc always, resulting
> in performance downgrade? I think this approach can make enhancement.

Well, it's not that easy. Then it's up to the freeing context to decide 
if a page should better be freed. Similarly, if you have a BUG (e.g., 
random put_page() from context X frees a user space page to the buddy) 
there, the whole security feature is - again - moot. That's why really 
only "init_on_free" vs "init_on_alloc" make sense - for anything that 
enters/leaves the buddy. As soon as you start poking holes you start 
opening the door for such security issues.

Enabling init_on_free has the downside that system boots gets slower, as 
everything that enters the buddy (== all memory) has to be zeroed out.

-- 
Thanks,

David / dhildenb



^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH] mm: add ___GFP_NOINIT flag which disables zeroing on alloc
  2021-03-29 10:12     ` Hyunsoon Kim
@ 2021-03-31  3:55       ` Leon Romanovsky
  0 siblings, 0 replies; 8+ messages in thread
From: Leon Romanovsky @ 2021-03-31  3:55 UTC (permalink / raw)
  To: Hyunsoon Kim; +Cc: Andrew Morton, dseok.yi, linux-mm, linux-kernel

On Mon, Mar 29, 2021 at 07:12:55PM +0900, Hyunsoon Kim wrote:
> On Mon, Mar 29, 2021 at 09:34:31AM +0300, Leon Romanovsky wrote:
> > On Mon, Mar 29, 2021 at 02:29:10PM +0900, Hyunsoon Kim wrote:
> > > This patch allows programmer to avoid zero initialization on page
> > > allocation even when the kernel config "CONFIG_INIT_ON_ALLOC_DEFAULT"
> > > is enabled. The configuration is made to prevent uninitialized
> > > heap memory flaws, and Android has applied this for security and
> > > deterministic execution times. Please refer to below.
> > > 
> > > https://android-review.googlesource.com/c/kernel/common/+/1235132
> > > 
> > > However, there is a case that the zeroing page memory is unnecessary
> > > when the page is used on specific purpose and will be zeroed
> > > automatically by hardware that accesses the memory through DMA.
> > > For instance, page allocation used for IP packet reception from Exynos
> > > modem is solely used for packet reception. Although the page will be
> > > freed eventually and reused for some other purpose, initialization at
> > > that moment of reuse will be sufficient to avoid uninitialized heap
> > > memory flaws. To support this kind of control, this patch creates new
> > > gfp type called ___GFP_NOINIT, that allows no zeroing at the moment
> > > of page allocation, called by many related APIs such as page_frag_alloc,
> > > alloc_pages, etc.
> > > 
> > > Signed-off-by: Hyunsoon Kim <h10.kim@samsung.com>
> > > ---
> > >  include/linux/gfp.h | 2 ++
> > >  include/linux/mm.h  | 4 +++-
> > >  2 files changed, 5 insertions(+), 1 deletion(-)
> > 
> > Let's assume that we will use this new flag, and users are smart enough
> > to figure when it needs to be used, what will be the performance gain?
> > 
> > Thanks
> 
> For instance, there are four memory access (either read or write) done
> by the system; memory write due to page allocation for reserving memory
> for modem hardware, memory write on the page by modem hardware,
> read and write incurred by copy_to_user operation by iperf reading
> the incoming network data. Theoretically, we can expect 1/4 of power
> saving on DRAM bandwidth. By performing simple iperf test with download
> UDP 800Mbps, we saw 5-6mA power gain by disabling
> CONFIG_INIT_ON_ALLOC_DEFAULT.

I'm more interested to see real results.

Thanks

> 
> Thanks




^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-03-31  3:56 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <CGME20210329054156epcas2p31650fa755e6cbcc55c4f33a79878256f@epcas2p3.samsung.com>
2021-03-29  5:29 ` [PATCH] mm: add ___GFP_NOINIT flag which disables zeroing on alloc Hyunsoon Kim
2021-03-29  6:34   ` Leon Romanovsky
2021-03-29 10:12     ` Hyunsoon Kim
2021-03-31  3:55       ` Leon Romanovsky
2021-03-29  9:47   ` kernel test robot
2021-03-29 10:53   ` David Hildenbrand
2021-03-30  1:44     ` Hyunsoon Kim
2021-03-30  8:52       ` David Hildenbrand

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).