Re: [PATCH 3/7] struct page: add field for vm_struct

[J Freyensee]

Reviewed-by: Jay Freyensee 

On 2/28/18 12:06 PM, Igor Stoppa wrote:

When a page is used for virtual memory, it is often necessary to obtain
a handler to the corresponding vm_struct, which refers to the virtually
continuous area generated when invoking vmalloc.

The struct page has a "mapping" field, which can be re-used, to store a
pointer to the parent area.

This will avoid more expensive searches, later on.

Signed-off-by: Igor Stoppa 
---
  include/linux/mm_types.h | 1 +
  mm/vmalloc.c | 2 ++
  2 files changed, 3 insertions(+)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index fd1af6b9591d..c3a4825e10c0 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -84,6 +84,7 @@ struct page {
void *s_mem;/* slab first object */
atomic_t compound_mapcount; /* first tail page */
/* page_deferred_list().next -- second tail page */
+   struct vm_struct *area;
};
  
  	/* Second double word */

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index ebff729cc956..61a1ca22b0f6 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1536,6 +1536,7 @@ static void __vunmap(const void *addr, int 
deallocate_pages)
struct page *page = area->pages[i];
  
  			BUG_ON(!page);

+   page->area = NULL;
__free_pages(page, 0);
}
  
@@ -1705,6 +1706,7 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,

area->nr_pages = i;
goto fail;
}
+   page->area = area;
area->pages[i] = page;
if (gfpflags_allow_blocking(gfp_mask|highmem_mask))
cond_resched();

Re: [PATCH 3/7] struct page: add field for vm_struct

[J Freyensee]

Reviewed-by: Jay Freyensee 

On 2/28/18 12:06 PM, Igor Stoppa wrote:

When a page is used for virtual memory, it is often necessary to obtain
a handler to the corresponding vm_struct, which refers to the virtually
continuous area generated when invoking vmalloc.

The struct page has a "mapping" field, which can be re-used, to store a
pointer to the parent area.

This will avoid more expensive searches, later on.

Signed-off-by: Igor Stoppa 
---
  include/linux/mm_types.h | 1 +
  mm/vmalloc.c | 2 ++
  2 files changed, 3 insertions(+)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index fd1af6b9591d..c3a4825e10c0 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -84,6 +84,7 @@ struct page {
void *s_mem;/* slab first object */
atomic_t compound_mapcount; /* first tail page */
/* page_deferred_list().next -- second tail page */
+   struct vm_struct *area;
};
  
  	/* Second double word */

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index ebff729cc956..61a1ca22b0f6 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1536,6 +1536,7 @@ static void __vunmap(const void *addr, int 
deallocate_pages)
struct page *page = area->pages[i];
  
  			BUG_ON(!page);

+   page->area = NULL;
__free_pages(page, 0);
}
  
@@ -1705,6 +1706,7 @@ static void *__vmalloc_area_node(struct vm_struct *area, gfp_t gfp_mask,

area->nr_pages = i;
goto fail;
}
+   page->area = area;
area->pages[i] = page;
if (gfpflags_allow_blocking(gfp_mask|highmem_mask))
cond_resched();

Re: [PATCH 3/7] struct page: add field for vm_struct

[kbuild test robot]

Hi Igor,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20180223]
[cannot apply to linus/master mmotm/master char-misc/char-misc-testing 
v4.16-rc3 v4.16-rc2 v4.16-rc1 v4.16-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Igor-Stoppa/mm-security-ro-protection-for-dynamic-data/20180302-232215
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   lib/genalloc.c:624:29: sparse: undefined identifier 'exit_test'
>> lib/genalloc.c:624:29: sparse: call with no type!
   In file included from arch/x86/include/asm/bug.h:83:0,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/gfp.h:5,
from include/linux/slab.h:15,
from lib/genalloc.c:99:
   lib/genalloc.c: In function 'gen_pool_free':
   lib/genalloc.c:616:10: warning: format '%s' expects argument of type 'char 
*', but argument 2 has type 'struct gen_pool *' [-Wformat=]
 "Trying to free unallocated memory"
 ^
   include/asm-generic/bug.h:98:50: note: in definition of macro '__WARN_printf'
#define __WARN_printf(arg...) do { __warn_printk(arg); __WARN(); } while (0)
 ^~~
   lib/genalloc.c:615:5: note: in expansion of macro 'WARN'
WARN(true,
^~~~
   lib/genalloc.c:617:23: note: format string is defined here
 " from pool %s", pool);
 ~^
   In file included from include/asm-generic/bug.h:5:0,
from arch/x86/include/asm/bug.h:83,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/gfp.h:5,
from include/linux/slab.h:15,
from lib/genalloc.c:99:
   lib/genalloc.c:624:17: error: implicit declaration of function 'exit_test'; 
did you mean 'exit_sem'? [-Werror=implicit-function-declaration]
   if (unlikely(exit_test(boundary < 0))) {
^
   include/linux/compiler.h:77:42: note: in definition of macro 'unlikely'
# define unlikely(x) __builtin_expect(!!(x), 0)
 ^
   In file included from arch/x86/include/asm/bug.h:83:0,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/gfp.h:5,
from include/linux/slab.h:15,
from lib/genalloc.c:99:
   lib/genalloc.c:626:16: warning: format '%s' expects argument of type 'char 
*', but argument 2 has type 'struct gen_pool *' [-Wformat=]
WARN(true, "Corrupted pool %s", pool);
   ^
   include/asm-generic/bug.h:98:50: note: in definition of macro '__WARN_printf'
#define __WARN_printf(arg...) do { __warn_printk(arg); __WARN(); } while (0)
 ^~~
   lib/genalloc.c:626:5: note: in expansion of macro 'WARN'
WARN(true, "Corrupted pool %s", pool);
^~~~
   lib/genalloc.c:634:10: warning: format '%s' expects argument of type 'char 
*', but argument 2 has type 'struct gen_pool *' [-Wformat=]
 "Size provided differs from size "
 ^
   include/asm-generic/bug.h:98:50: note: in definition of macro '__WARN_printf'
#define __WARN_printf(arg...) do { __warn_printk(arg); __WARN(); } while (0)
 ^~~
   lib/genalloc.c:633:5: note: in expansion of macro 'WARN'
WARN(true,
^~~~
   lib/genalloc.c:635:31: note: format string is defined here
 "measured from pool %s", pool);
 ~^
   In file included from arch/x86/include/asm/bug.h:83:0,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/gfp.h:5,
from include/linux/slab.h:15,
from lib/genalloc.c:99:
   lib/genalloc.c:643:10: warning: format '%s' expects argument of type 'char 
*', but argument 2 has type 'struct gen_pool *' [-Wformat=]
 "Unexpected bitmap collision while"
 ^
   include/asm-generic/bug.h:98:50: note: in definition of macro '__WARN_printf'
#define __WARN_printf(arg...) do { __warn_printk(arg); __WARN(); } while (0)
 ^~~
   lib/genalloc.c:642:5: note: in expansion of macro 'WARN'
WARN(true,
^~~~
   lib/genalloc.c:644:36: note: format string is defined here
 " freeing memory in pool %s", pool);
  ~^
   cc1: some warnings being treated as errors

vim +624 

Re: [PATCH 3/7] struct page: add field for vm_struct

[kbuild test robot]

Hi Igor,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on next-20180223]
[cannot apply to linus/master mmotm/master char-misc/char-misc-testing 
v4.16-rc3 v4.16-rc2 v4.16-rc1 v4.16-rc3]
[if your patch is applied to the wrong git tree, please drop us a note to help 
improve the system]

url:
https://github.com/0day-ci/linux/commits/Igor-Stoppa/mm-security-ro-protection-for-dynamic-data/20180302-232215
reproduce:
# apt-get install sparse
make ARCH=x86_64 allmodconfig
make C=1 CF=-D__CHECK_ENDIAN__


sparse warnings: (new ones prefixed by >>)

   lib/genalloc.c:624:29: sparse: undefined identifier 'exit_test'
>> lib/genalloc.c:624:29: sparse: call with no type!
   In file included from arch/x86/include/asm/bug.h:83:0,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/gfp.h:5,
from include/linux/slab.h:15,
from lib/genalloc.c:99:
   lib/genalloc.c: In function 'gen_pool_free':
   lib/genalloc.c:616:10: warning: format '%s' expects argument of type 'char 
*', but argument 2 has type 'struct gen_pool *' [-Wformat=]
 "Trying to free unallocated memory"
 ^
   include/asm-generic/bug.h:98:50: note: in definition of macro '__WARN_printf'
#define __WARN_printf(arg...) do { __warn_printk(arg); __WARN(); } while (0)
 ^~~
   lib/genalloc.c:615:5: note: in expansion of macro 'WARN'
WARN(true,
^~~~
   lib/genalloc.c:617:23: note: format string is defined here
 " from pool %s", pool);
 ~^
   In file included from include/asm-generic/bug.h:5:0,
from arch/x86/include/asm/bug.h:83,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/gfp.h:5,
from include/linux/slab.h:15,
from lib/genalloc.c:99:
   lib/genalloc.c:624:17: error: implicit declaration of function 'exit_test'; 
did you mean 'exit_sem'? [-Werror=implicit-function-declaration]
   if (unlikely(exit_test(boundary < 0))) {
^
   include/linux/compiler.h:77:42: note: in definition of macro 'unlikely'
# define unlikely(x) __builtin_expect(!!(x), 0)
 ^
   In file included from arch/x86/include/asm/bug.h:83:0,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/gfp.h:5,
from include/linux/slab.h:15,
from lib/genalloc.c:99:
   lib/genalloc.c:626:16: warning: format '%s' expects argument of type 'char 
*', but argument 2 has type 'struct gen_pool *' [-Wformat=]
WARN(true, "Corrupted pool %s", pool);
   ^
   include/asm-generic/bug.h:98:50: note: in definition of macro '__WARN_printf'
#define __WARN_printf(arg...) do { __warn_printk(arg); __WARN(); } while (0)
 ^~~
   lib/genalloc.c:626:5: note: in expansion of macro 'WARN'
WARN(true, "Corrupted pool %s", pool);
^~~~
   lib/genalloc.c:634:10: warning: format '%s' expects argument of type 'char 
*', but argument 2 has type 'struct gen_pool *' [-Wformat=]
 "Size provided differs from size "
 ^
   include/asm-generic/bug.h:98:50: note: in definition of macro '__WARN_printf'
#define __WARN_printf(arg...) do { __warn_printk(arg); __WARN(); } while (0)
 ^~~
   lib/genalloc.c:633:5: note: in expansion of macro 'WARN'
WARN(true,
^~~~
   lib/genalloc.c:635:31: note: format string is defined here
 "measured from pool %s", pool);
 ~^
   In file included from arch/x86/include/asm/bug.h:83:0,
from include/linux/bug.h:5,
from include/linux/mmdebug.h:5,
from include/linux/gfp.h:5,
from include/linux/slab.h:15,
from lib/genalloc.c:99:
   lib/genalloc.c:643:10: warning: format '%s' expects argument of type 'char 
*', but argument 2 has type 'struct gen_pool *' [-Wformat=]
 "Unexpected bitmap collision while"
 ^
   include/asm-generic/bug.h:98:50: note: in definition of macro '__WARN_printf'
#define __WARN_printf(arg...) do { __warn_printk(arg); __WARN(); } while (0)
 ^~~
   lib/genalloc.c:642:5: note: in expansion of macro 'WARN'
WARN(true,
^~~~
   lib/genalloc.c:644:36: note: format string is defined here
 " freeing memory in pool %s", pool);
  ~^
   cc1: some warnings being treated as errors

vim +624 

Re: [PATCH 3/7] struct page: add field for vm_struct

[Igor Stoppa]

On 25/02/18 05:38, Matthew Wilcox wrote:
> On Fri, Feb 23, 2018 at 04:48:03PM +0200, Igor Stoppa wrote:
>> @@ -1769,6 +1771,9 @@ void *__vmalloc_node_range(unsigned long size, 
>> unsigned long align,
>>  
>>  kmemleak_vmalloc(area, size, gfp_mask);
>>  
>> +for (i = 0; i < area->nr_pages; i++)
>> +area->pages[i]->area = area;
>> +
>>  return addr;
>>  
>>  fail:
> 
> IMO, this is the wrong place to initialise the page->area.  It should be
> done in __vmalloc_area_node() like so:
> 
> area->nr_pages = i;
> goto fail;
> }
> + page->area = area;
> area->pages[i] = page;
> if (gfpflags_allow_blocking(gfp_mask))
> cond_resched();
> 


ok

--
igor

Re: [PATCH 3/7] struct page: add field for vm_struct

[Igor Stoppa]

On 25/02/18 05:38, Matthew Wilcox wrote:
> On Fri, Feb 23, 2018 at 04:48:03PM +0200, Igor Stoppa wrote:
>> @@ -1769,6 +1771,9 @@ void *__vmalloc_node_range(unsigned long size, 
>> unsigned long align,
>>  
>>  kmemleak_vmalloc(area, size, gfp_mask);
>>  
>> +for (i = 0; i < area->nr_pages; i++)
>> +area->pages[i]->area = area;
>> +
>>  return addr;
>>  
>>  fail:
> 
> IMO, this is the wrong place to initialise the page->area.  It should be
> done in __vmalloc_area_node() like so:
> 
> area->nr_pages = i;
> goto fail;
> }
> + page->area = area;
> area->pages[i] = page;
> if (gfpflags_allow_blocking(gfp_mask))
> cond_resched();
> 


ok

--
igor

Re: [PATCH 3/7] struct page: add field for vm_struct

[Matthew Wilcox]

On Fri, Feb 23, 2018 at 04:48:03PM +0200, Igor Stoppa wrote:
> @@ -1769,6 +1771,9 @@ void *__vmalloc_node_range(unsigned long size, unsigned 
> long align,
>  
>   kmemleak_vmalloc(area, size, gfp_mask);
>  
> + for (i = 0; i < area->nr_pages; i++)
> + area->pages[i]->area = area;
> +
>   return addr;
>  
>  fail:

IMO, this is the wrong place to initialise the page->area.  It should be
done in __vmalloc_area_node() like so:

area->nr_pages = i;
goto fail;
}
+   page->area = area;
area->pages[i] = page;
if (gfpflags_allow_blocking(gfp_mask))
cond_resched();

Re: [PATCH 3/7] struct page: add field for vm_struct

[Matthew Wilcox]

On Fri, Feb 23, 2018 at 04:48:03PM +0200, Igor Stoppa wrote:
> @@ -1769,6 +1771,9 @@ void *__vmalloc_node_range(unsigned long size, unsigned 
> long align,
>  
>   kmemleak_vmalloc(area, size, gfp_mask);
>  
> + for (i = 0; i < area->nr_pages; i++)
> + area->pages[i]->area = area;
> +
>   return addr;
>  
>  fail:

IMO, this is the wrong place to initialise the page->area.  It should be
done in __vmalloc_area_node() like so:

area->nr_pages = i;
goto fail;
}
+   page->area = area;
area->pages[i] = page;
if (gfpflags_allow_blocking(gfp_mask))
cond_resched();