Re: [PATCH] mm: __nr_to_section - make it safe against overflow v2

Andrew Morton
2009-01-06T00:38:37Z

On Mon, 5 Jan 2009 13:31:32 +0300
Cyrill Gorcunov <gorcunov@gmail.com> wrote:

> __nr_to_section should check for array bound overflow.
> We should better get NULL dereference then silently
> pass some memory snippet out of bounds to a caller.
>

Are there actually any known problems here?

>
> Signed-off-by: Cyrill Gorcunov <gorcunov@openvz.org>
> ---
> include/linux/mmzone.h | 15 +++++++++++++--
> 1 file changed, 13 insertions(+), 2 deletions(-)
>
> Index: linux-2.6.git/include/linux/mmzone.h
> ===================================================================
> --- linux-2.6.git.orig/include/linux/mmzone.h
> +++ linux-2.6.git/include/linux/mmzone.h
> @@ -935,6 +935,12 @@ static inline unsigned long early_pfn_to
>
> struct page;
> struct page_cgroup;
> +
> +/*
> + * NOTE: sizeof(struct mem_section) _must_ be power of 2
> + * otherwise SECTION_ROOT_MASK will be broken so be
> + * really cautious while modifying this structure
> + */
> struct mem_section {
> /*
> * This is, logically, a pointer to an array of struct
> @@ -980,9 +986,14 @@ extern struct mem_section mem_section[NR
>
> static inline struct mem_section *__nr_to_section(unsigned long nr)
> {
> - if (!mem_section[SECTION_NR_TO_ROOT(nr)])
> + unsigned long idx = SECTION_NR_TO_ROOT(nr);
> +
> + if (WARN_ON(idx >= NR_SECTION_ROOTS))
> + return NULL;
> +
> + if (!mem_section[idx])
> return NULL;
> - return &mem_section[SECTION_NR_TO_ROOT(nr)][nr & SECTION_ROOT_MASK];
> + return &mem_section[idx][nr & SECTION_ROOT_MASK];
> }

The patch adds nearly 300 bytes of stuff to mm/sparse.o, and for what??