From: Quanyang Wang <[email protected]>
If using "vend >= begin" to judge if two memory regions intersects, vend
should be the end of the memory region, so it should be "virt + size -1"
instead of "virt + size".
The wrong determination of the end triggers the misreporting as below when
the dma debug function "check_for_illegal_area" calls memory_intersects to
check if the dma region intersects with stext region.
Calltrace (stext is at 0x80100000):
WARNING: CPU: 0 PID: 77 at kernel/dma/debug.c:1073 check_for_illegal_area+0x130/0x168
DMA-API: chipidea-usb2 e0002000.usb: device driver maps memory from kernel text or rodata [addr=800f0000] [len=65536]
Modules linked in:
CPU: 1 PID: 77 Comm: usb-storage Not tainted 5.19.0-yocto-standard #5
Hardware name: Xilinx Zynq Platform
unwind_backtrace from show_stack+0x18/0x1c
show_stack from dump_stack_lvl+0x58/0x70
dump_stack_lvl from __warn+0xb0/0x198
__warn from warn_slowpath_fmt+0x80/0xb4
warn_slowpath_fmt from check_for_illegal_area+0x130/0x168
check_for_illegal_area from debug_dma_map_sg+0x94/0x368
debug_dma_map_sg from __dma_map_sg_attrs+0x114/0x128
__dma_map_sg_attrs from dma_map_sg_attrs+0x18/0x24
dma_map_sg_attrs from usb_hcd_map_urb_for_dma+0x250/0x3b4
usb_hcd_map_urb_for_dma from usb_hcd_submit_urb+0x194/0x214
usb_hcd_submit_urb from usb_sg_wait+0xa4/0x118
usb_sg_wait from usb_stor_bulk_transfer_sglist+0xa0/0xec
usb_stor_bulk_transfer_sglist from usb_stor_bulk_srb+0x38/0x70
usb_stor_bulk_srb from usb_stor_Bulk_transport+0x150/0x360
usb_stor_Bulk_transport from usb_stor_invoke_transport+0x38/0x440
usb_stor_invoke_transport from usb_stor_control_thread+0x1e0/0x238
usb_stor_control_thread from kthread+0xf8/0x104
kthread from ret_from_fork+0x14/0x2c
Fixes: 979559362516 ("asm/sections: add helpers to check for section data")
Signed-off-by: Quanyang Wang <[email protected]>
---
include/asm-generic/sections.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
index d0f7bdd2fdf2..f7171b4f5bfd 100644
--- a/include/asm-generic/sections.h
+++ b/include/asm-generic/sections.h
@@ -108,7 +108,7 @@ static inline bool memory_contains(void *begin, void *end, void *virt,
static inline bool memory_intersects(void *begin, void *end, void *virt,
size_t size)
{
- void *vend = virt + size;
+ void *vend = virt + size - 1;
return (virt >= begin && virt < end) || (vend >= begin && vend < end);
}
--
2.36.1
On Thu, 11 Aug 2022 at 08:31, <[email protected]> wrote:
>
> From: Quanyang Wang <[email protected]>
>
> If using "vend >= begin" to judge if two memory regions intersects, vend
> should be the end of the memory region, so it should be "virt + size -1"
> instead of "virt + size".
> The wrong determination of the end triggers the misreporting as below when
> the dma debug function "check_for_illegal_area" calls memory_intersects to
> check if the dma region intersects with stext region.
>
> Calltrace (stext is at 0x80100000):
> WARNING: CPU: 0 PID: 77 at kernel/dma/debug.c:1073 check_for_illegal_area+0x130/0x168
> DMA-API: chipidea-usb2 e0002000.usb: device driver maps memory from kernel text or rodata [addr=800f0000] [len=65536]
> Modules linked in:
> CPU: 1 PID: 77 Comm: usb-storage Not tainted 5.19.0-yocto-standard #5
> Hardware name: Xilinx Zynq Platform
> unwind_backtrace from show_stack+0x18/0x1c
> show_stack from dump_stack_lvl+0x58/0x70
> dump_stack_lvl from __warn+0xb0/0x198
> __warn from warn_slowpath_fmt+0x80/0xb4
> warn_slowpath_fmt from check_for_illegal_area+0x130/0x168
> check_for_illegal_area from debug_dma_map_sg+0x94/0x368
> debug_dma_map_sg from __dma_map_sg_attrs+0x114/0x128
> __dma_map_sg_attrs from dma_map_sg_attrs+0x18/0x24
> dma_map_sg_attrs from usb_hcd_map_urb_for_dma+0x250/0x3b4
> usb_hcd_map_urb_for_dma from usb_hcd_submit_urb+0x194/0x214
> usb_hcd_submit_urb from usb_sg_wait+0xa4/0x118
> usb_sg_wait from usb_stor_bulk_transfer_sglist+0xa0/0xec
> usb_stor_bulk_transfer_sglist from usb_stor_bulk_srb+0x38/0x70
> usb_stor_bulk_srb from usb_stor_Bulk_transport+0x150/0x360
> usb_stor_Bulk_transport from usb_stor_invoke_transport+0x38/0x440
> usb_stor_invoke_transport from usb_stor_control_thread+0x1e0/0x238
> usb_stor_control_thread from kthread+0xf8/0x104
> kthread from ret_from_fork+0x14/0x2c
>
> Fixes: 979559362516 ("asm/sections: add helpers to check for section data")
> Signed-off-by: Quanyang Wang <[email protected]>
> ---
> include/asm-generic/sections.h | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
> index d0f7bdd2fdf2..f7171b4f5bfd 100644
> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -108,7 +108,7 @@ static inline bool memory_contains(void *begin, void *end, void *virt,
> static inline bool memory_intersects(void *begin, void *end, void *virt,
> size_t size)
> {
> - void *vend = virt + size;
> + void *vend = virt + size - 1;
>
> return (virt >= begin && virt < end) || (vend >= begin && vend < end);
This test looks flawed to me for another reason as well: it only
checks whether the start /or/ the end of (virt, virt+size) falls
inside the area, so if the area is covered completely (in which case
the intersection of the two will be equal to the area), this will
return false erroneously.
Hi Ard,
On 8/11/22 16:16, Ard Biesheuvel wrote:
> On Thu, 11 Aug 2022 at 08:31, <[email protected]> wrote:
>>
>> From: Quanyang Wang <[email protected]>
>>
>> If using "vend >= begin" to judge if two memory regions intersects, vend
>> should be the end of the memory region, so it should be "virt + size -1"
>> instead of "virt + size".
>> The wrong determination of the end triggers the misreporting as below when
>> the dma debug function "check_for_illegal_area" calls memory_intersects to
>> check if the dma region intersects with stext region.
>>
>> Calltrace (stext is at 0x80100000):
>> WARNING: CPU: 0 PID: 77 at kernel/dma/debug.c:1073 check_for_illegal_area+0x130/0x168
>> DMA-API: chipidea-usb2 e0002000.usb: device driver maps memory from kernel text or rodata [addr=800f0000] [len=65536]
>> Modules linked in:
>> CPU: 1 PID: 77 Comm: usb-storage Not tainted 5.19.0-yocto-standard #5
>> Hardware name: Xilinx Zynq Platform
>> unwind_backtrace from show_stack+0x18/0x1c
>> show_stack from dump_stack_lvl+0x58/0x70
>> dump_stack_lvl from __warn+0xb0/0x198
>> __warn from warn_slowpath_fmt+0x80/0xb4
>> warn_slowpath_fmt from check_for_illegal_area+0x130/0x168
>> check_for_illegal_area from debug_dma_map_sg+0x94/0x368
>> debug_dma_map_sg from __dma_map_sg_attrs+0x114/0x128
>> __dma_map_sg_attrs from dma_map_sg_attrs+0x18/0x24
>> dma_map_sg_attrs from usb_hcd_map_urb_for_dma+0x250/0x3b4
>> usb_hcd_map_urb_for_dma from usb_hcd_submit_urb+0x194/0x214
>> usb_hcd_submit_urb from usb_sg_wait+0xa4/0x118
>> usb_sg_wait from usb_stor_bulk_transfer_sglist+0xa0/0xec
>> usb_stor_bulk_transfer_sglist from usb_stor_bulk_srb+0x38/0x70
>> usb_stor_bulk_srb from usb_stor_Bulk_transport+0x150/0x360
>> usb_stor_Bulk_transport from usb_stor_invoke_transport+0x38/0x440
>> usb_stor_invoke_transport from usb_stor_control_thread+0x1e0/0x238
>> usb_stor_control_thread from kthread+0xf8/0x104
>> kthread from ret_from_fork+0x14/0x2c
>>
>> Fixes: 979559362516 ("asm/sections: add helpers to check for section data")
>> Signed-off-by: Quanyang Wang <[email protected]>
>> ---
>> include/asm-generic/sections.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/asm-generic/sections.h b/include/asm-generic/sections.h
>> index d0f7bdd2fdf2..f7171b4f5bfd 100644
>> --- a/include/asm-generic/sections.h
>> +++ b/include/asm-generic/sections.h
>> @@ -108,7 +108,7 @@ static inline bool memory_contains(void *begin, void *end, void *virt,
>> static inline bool memory_intersects(void *begin, void *end, void *virt,
>> size_t size)
>> {
>> - void *vend = virt + size;
>> + void *vend = virt + size - 1;
>>
>> return (virt >= begin && virt < end) || (vend >= begin && vend < end);
>
> This test looks flawed to me for another reason as well: it only
> checks whether the start /or/ the end of (virt, virt+size) falls
> inside the area, so if the area is covered completely (in which case
> the intersection of the two will be equal to the area), this will
> return false erroneously.
Yes, the test lacks some checks. I will send a V2 patch to fix it.
Thanks,
Quanyang