cma: check for memory region overlapping
Subsystems (2)
PATCH 1/1 cma: check for memory region overlapping
From: Binglei Wang <[email protected]>
Cma memory region editted carelessly in dts may overlap with kernel code/data memory region which is reserved by memblock during the early phase of system memory initialization.
Without checking overlap and cma area setup done, this region will be released to buddy system later.
When memory usage under pressure, memory allocated from this region will collide with kernel code which is read-only. And the following writing to this region will trigger the panic of writing to read-only memory.
So when rmem_cma_setup returns EBUSY, do not phys-free this region to memblock or else we end up with free the kernel code memory to buddy system.
--- drivers/of/of_reserved_mem.c | 3 --- kernel/dma/contiguous.c | 5 +++++ 2 files changed, 5 insertions(+), 3 deletions(-)
drivers/of/of_reserved_mem.c +0 -3
@@ -338,9 +338,6 @@ void __init fdt_init_reserved_mem(void) rmem->name);if (nomap)memblock_clear_nomap (rmem->base, rmem->size); -else -memblock_phys_free (rmem->base, - rmem->size); }else {phys_addr_t end = rmem->base + rmem->size -1 ; bool reusable =
kernel/dma/contiguous.c +5 -0
@@ -410,6 +410,11 @@ static int __init rmem_cma_setup(struct reserved_mem *rmem)return -EBUSY ; } +if (memblock_is_region_reserved (rmem->base, rmem->size)) { +pr_info ("Reserved memory: overlap with exsiting one\n" ); +return -EBUSY ; + } +if (!of_get_flat_dt_prop (node,"reusable" ,NULL ) ||of_get_flat_dt_prop (node,"no-map" ,NULL ))return -EINVAL ;
Signature · 1 line
2.34.1
Rob Herring
maintainerQuoted 5 lines · Binglei Wang
From: Binglei Wang <[email protected]>
Cma memory region editted carelessly in dts may overlap with kernel code/data memory region which is reserved by memblock during the early phase of system memory initialization.
Is your goal for the kernel to function with this careless editing or warn enough to fix the DT?
What if other regions overlap with the kernel? Wouldn't we have the same problem?
Quoted 24 lines
Without checking overlap and cma area setup done, this region will be released to buddy system later.
When memory usage under pressure, memory allocated from this region will collide with kernel code which is read-only. And the following writing to this region will trigger the panic of writing to read-only memory.
So when
rmem_cma_setupreturns EBUSY, do not phys-free this region to memblock or else we end up with free the kernel code memory to buddy system.Signed-off-by Binglei Wang <[email protected]>--- drivers/of/of_reserved_mem.c | 3 --- kernel/dma/contiguous.c | 5 +++++ 2 files changed, 5 insertions(+), 3 deletions(-)drivers/of/of_reserved_mem.c +0
-0
drivers/of/of_reserved_mem.c void __init fdt_init_reserved_mem(void) · line 343
Quoted 6 lines
drivers/of/of_reserved_mem.c +0
-3@@ -338,9 +338,6 @@ void __init fdt_init_reserved_mem(void) rmem->name);if (nomap)memblock_clear_nomap (rmem->base, rmem->size); -else -memblock_phys_free (rmem->base, - rmem->size);
I don't understand this change. Seems like perhaps someone would want the free here?
kernel/dma/contiguous.c static int __init rmem_cma_setup(struct reserved_mem *rmem) · line 410
Quoted 9 lines
drivers/of/of_reserved_mem.c +0
-0}else {phys_addr_t end = rmem->base + rmem->size -1 ; bool reusable =kernel/dma/contiguous.c +0
-0@@ -410,6 +410,11 @@ static int __init rmem_cma_setup(struct reserved_mem *rmem)return -EBUSY ;
Here we return EBUSY if we are using cmdline params. In that case, isn't freeing the DT CMA area the right thing to do?
Quoted 13 lines
kernel/dma/contiguous.c +5
-0} +if (memblock_is_region_reserved (rmem->base, rmem->size)) { +pr_info ("Reserved memory: overlap with exsiting one\n" ); +return -EBUSY ; + } +if (!of_get_flat_dt_prop (node,"reusable" ,NULL ) ||of_get_flat_dt_prop (node,"no-map" ,NULL ))return -EINVAL ;Signature · 1 line
2.34.1
Quoted 8 lines · Rob Herring <[email protected]> 于2023年7月22日周六 00:21写道：
Quoted 5 lines · Binglei Wang
From: Binglei Wang <[email protected]>
Cma memory region editted carelessly in dts may overlap with kernel code/data memory region which is reserved by memblock during the early phase of system memory initialization.
Quoted 15 lines
Is your goal for the kernel to function with this careless editing or warn enough to fix the DT?
What if other regions overlap with the kernel? Wouldn't we have the same problem?
Quoted 9 lines
Without checking overlap and cma area setup done, this region will be released to buddy system later.
When memory usage under pressure, memory allocated from this region will collide with kernel code which is read-only. And the following writing to this region will trigger the panic of writing to read-only memory.
Dear Rob:
Thanks for your reply.
In the production environment, I encountered this issue in the 5.4 kernel. The goal is just to warn enough to fix the DT.
Quoted 25 lines
Quoted 15 lines
So when
rmem_cma_setupreturns EBUSY, do not phys-free this region to memblock or else we end up with free the kernel code memory to buddy system.Signed-off-by Binglei Wang <[email protected]>--- drivers/of/of_reserved_mem.c | 3 --- kernel/dma/contiguous.c | 5 +++++ 2 files changed, 5 insertions(+), 3 deletions(-)drivers/of/of_reserved_mem.c +0
-0drivers/of/of_reserved_mem.c void __init fdt_init_reserved_mem(void) · line 343
Quoted 6 lines
drivers/of/of_reserved_mem.c +0
-3@@ -338,9 +338,6 @@ void __init fdt_init_reserved_mem(void) rmem->name);if (nomap)memblock_clear_nomap (rmem->base, rmem->size); -else -memblock_phys_free (rmem->base, - rmem->size);I don't understand this change. Seems like perhaps someone would want the free here?
Quoted 14 lines
kernel/dma/contiguous.c static int __init rmem_cma_setup(struct reserved_mem *rmem) · line 410
Quoted 9 lines
drivers/of/of_reserved_mem.c +0
-0}else {phys_addr_t end = rmem->base + rmem->size -1 ; bool reusable =kernel/dma/contiguous.c +0
-0@@ -410,6 +410,11 @@ static int __init rmem_cma_setup(struct reserved_mem *rmem)return -EBUSY ;Here we return EBUSY if we are using cmdline params. In that case, isn't freeing the DT CMA area the right thing to do?
yes, in some cases, freeing the DT CMA area is the right thing to do. in another case, like the read-only panic, freeing will cause problems.
Actually, giving a warning message is already sufficient. I should not overthink it, after all, it was caused by human factors.
When using command line,
I find cma_declare_contiguous_nid will check the overlap in the latest kernel. But in the program flow of parsing dts, both the 5.4 kernel and the latest version of the kernel lack this check.
That's the reason for this patch.
Quoted 6 lines
kernel/dma/contiguous.c static int __init rmem_cma_setup(struct reserved_mem *rmem) · line 416
Quoted 6 lines
kernel/dma/contiguous.c +4
-0} +if (memblock_is_region_reserved (rmem->base, rmem->size)) { +pr_info ("Reserved memory: overlap with exsiting one\n" ); +return -EBUSY ; + }
Do you think a warning message is sufficient?
Best wishes
Binglei Wang.
Quoted 7 lines
Quoted 7 lines
kernel/dma/contiguous.c +1
-1+if (!of_get_flat_dt_prop (node,"reusable" ,NULL ) ||of_get_flat_dt_prop (node,"no-map" ,NULL ))return -EINVAL ; --2.34.1