* [PATCH] asm/sections: fix the determination of the end of the memory region
@ 2022-08-11 6:31 quanyang.wang
2022-08-11 8:16 ` Ard Biesheuvel
0 siblings, 1 reply; 3+ messages in thread
From: quanyang.wang @ 2022-08-11 6:31 UTC (permalink / raw
To: Arnd Bergmann, Andrew Morton, Thierry Reding
Cc: linux-arch, linux-kernel, Quanyang Wang
From: Quanyang Wang <quanyang.wang@windriver.com>
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 <quanyang.wang@windriver.com>
---
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] asm/sections: fix the determination of the end of the memory region
2022-08-11 6:31 [PATCH] asm/sections: fix the determination of the end of the memory region quanyang.wang
@ 2022-08-11 8:16 ` Ard Biesheuvel
2022-08-16 1:00 ` Quanyang Wang
0 siblings, 1 reply; 3+ messages in thread
From: Ard Biesheuvel @ 2022-08-11 8:16 UTC (permalink / raw
To: quanyang.wang
Cc: Arnd Bergmann, Andrew Morton, Thierry Reding, linux-arch,
linux-kernel
On Thu, 11 Aug 2022 at 08:31, <quanyang.wang@windriver.com> wrote:
>
> From: Quanyang Wang <quanyang.wang@windriver.com>
>
> 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 <quanyang.wang@windriver.com>
> ---
> 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.
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] asm/sections: fix the determination of the end of the memory region
2022-08-11 8:16 ` Ard Biesheuvel
@ 2022-08-16 1:00 ` Quanyang Wang
0 siblings, 0 replies; 3+ messages in thread
From: Quanyang Wang @ 2022-08-16 1:00 UTC (permalink / raw
To: Ard Biesheuvel
Cc: Arnd Bergmann, Andrew Morton, Thierry Reding, linux-arch,
linux-kernel
Hi Ard,
On 8/11/22 16:16, Ard Biesheuvel wrote:
> On Thu, 11 Aug 2022 at 08:31, <quanyang.wang@windriver.com> wrote:
>>
>> From: Quanyang Wang <quanyang.wang@windriver.com>
>>
>> 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 <quanyang.wang@windriver.com>
>> ---
>> 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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-08-16 4:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-08-11 6:31 [PATCH] asm/sections: fix the determination of the end of the memory region quanyang.wang
2022-08-11 8:16 ` Ard Biesheuvel
2022-08-16 1:00 ` Quanyang Wang
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).