* Re: [patch 066/134] mm/memory_hotplug.c: add a bounds check to check_hotplug_memory_range()
[not found] <20190923223612.gJ-_DTlay%akpm@linux-foundation.org>
@ 2019-09-24 7:47 ` Michal Hocko
2019-09-24 22:38 ` Linus Torvalds
0 siblings, 1 reply; 2+ messages in thread
From: Michal Hocko @ 2019-09-24 7:47 UTC (permalink / raw)
To: linux-kernel
Cc: akpm, alastair, cai, dan.j.williams, david, ira.weiny, jgg,
logang, mm-commits, osalvador, pasha.tatashin, richard.weiyang,
torvalds
On Mon 23-09-19 15:36:12, Andrew Morton wrote:
> From: Alastair D'Silva <alastair@d-silva.org>
> Subject: mm/memory_hotplug.c: add a bounds check to check_hotplug_memory_range()
>
> Patch series "Add bounds check for Hotplugged memory", v3.
>
> This series adds bounds checks for hotplugged memory, ensuring that it is
> within the physically addressable range (for platforms that define
> MAX_(POSSIBLE_)PHYSMEM_BITS.
>
> This allows for early failure, rather than attempting to access bogus
> section numbers.
>
>
> This patch (of 2):
>
> On PowerPC, the address ranges allocated to OpenCAPI LPC memory are
> allocated from firmware. These address ranges may be higher than what
> older kernels permit, as we increased the maximum permissable address in
> commit 4ffe713b7587 ("powerpc/mm: Increase the max addressable memory to
> 2PB"). It is possible that the addressable range may change again in the
> future.
>
> In this scenario, we end up with a bogus section returned from
> __section_nr (see the discussion on the thread "mm: Trigger bug on if a
> section is not found in __section_nr").
>
> Adding a check here means that we fail early and have an opportunity to
> handle the error gracefully, rather than rumbling on and potentially
> accessing an incorrect section.
>
> Further discussion is also on the thread ("powerpc: Perform a bounds check
> in arch_add_memory").
>
> David said:
>
> : I guess checking for address space wrapping would be overkill. This
> : change makes sense for architecture-independent device drivers that make
> : use of the add/remove memory infrastructure (e.g., virtio-mem I am working
> : on).
Isn't this rushed a bit? I've had a review feedback just yesterday
http://lkml.kernel.org/r/20190923122513.GO6016@dhcp22.suse.cz and had
some concerns.
> Link: http://lkml.kernel.org/r/20190917010752.28395-2-alastair@au1.ibm.com
> Signed-off-by: Alastair D'Silva <alastair@d-silva.org>
> Reviewed-by: David Hildenbrand <david@redhat.com>
> Cc: Oscar Salvador <osalvador@suse.com>
> Cc: Michal Hocko <mhocko@suse.com>
> Cc: Pavel Tatashin <pasha.tatashin@soleen.com>
> Cc: Dan Williams <dan.j.williams@intel.com>
> Cc: Wei Yang <richard.weiyang@gmail.com>
> Cc: Qian Cai <cai@lca.pw>
> Cc: Jason Gunthorpe <jgg@ziepe.ca>
> Cc: Logan Gunthorpe <logang@deltatee.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
> ---
>
> include/linux/memory_hotplug.h | 1 +
> mm/memory_hotplug.c | 13 ++++++++++++-
> 2 files changed, 13 insertions(+), 1 deletion(-)
>
> --- a/include/linux/memory_hotplug.h~memory_hotplug-add-a-bounds-check-to-check_hotplug_memory_range
> +++ a/include/linux/memory_hotplug.h
> @@ -110,6 +110,7 @@ extern void __online_page_increment_coun
> extern void __online_page_free(struct page *page);
>
> extern int try_online_node(int nid);
> +int check_hotplug_memory_addressable(u64 start, u64 size);
>
> extern int arch_add_memory(int nid, u64 start, u64 size,
> struct mhp_restrictions *restrictions);
> --- a/mm/memory_hotplug.c~memory_hotplug-add-a-bounds-check-to-check_hotplug_memory_range
> +++ a/mm/memory_hotplug.c
> @@ -1023,6 +1023,17 @@ int try_online_node(int nid)
> return ret;
> }
>
> +int check_hotplug_memory_addressable(u64 start, u64 size)
> +{
> +#ifdef MAX_PHYSMEM_BITS
> + if ((start + size - 1) >> MAX_PHYSMEM_BITS)
> + return -E2BIG;
> +#endif
> +
> + return 0;
> +}
> +EXPORT_SYMBOL_GPL(check_hotplug_memory_addressable);
> +
> static int check_hotplug_memory_range(u64 start, u64 size)
> {
> /* memory range must be block size aligned */
> @@ -1033,7 +1044,7 @@ static int check_hotplug_memory_range(u6
> return -EINVAL;
> }
>
> - return 0;
> + return check_hotplug_memory_addressable(start, size);
> }
>
> static int online_memory_block(struct memory_block *mem, void *arg)
> _
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 2+ messages in thread