lkml.org 
[lkml]   [2022]   [Aug]   [19]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
SubjectRe: [V2][PATCH] asm-generic: sections: refactor memory_intersects
On Fri, 19 Aug 2022 16:11:45 +0800 quanyang.wang@windriver.com wrote:

> From: Quanyang Wang <quanyang.wang@windriver.com>
>
> There are two problems with the current code of memory_intersects:
>
> First, it doesn't check whether the region (begin, end) falls inside
> the region (virt, vend), that is (virt < begin && vend > end).
>
> The second problem is if vend is equal to begin, it will return true
> but this is wrong since vend (virt + size) is not the last address of
> the memory region but (virt + size -1) is. The wrong determination will
> trigger the misreporting when the function check_for_illegal_area calls
> memory_intersects to check if the dma region intersects with stext region.
>
> The misreporting is as below (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
>
> Refactor memory_intersects to fix the two problems above.
>
> ...

There must be tons of places in the kernel which check to see if two
regions overlap at all, I'm not sure why dma debug needs its own one?

> --- a/include/asm-generic/sections.h
> +++ b/include/asm-generic/sections.h
> @@ -110,7 +110,10 @@ static inline bool memory_intersects(void *begin, void *end, void *virt,
> {
> void *vend = virt + size;
>
> - return (virt >= begin && virt < end) || (vend >= begin && vend < end);
> + if (virt < end && vend > begin)
> + return true;
> +
> + return false;
> }

These things bend my brain, but all the cases I've mind-tested worked
out OK.

Now the forever question: is a -stable backport needed? The bug
appears to be six years old, so I guess not. Can you suggest why it
took this long? Are you doing something unusual?


While we're in there, I can't resist fixing that typo...

--- a/include/asm-generic/sections.h~asm-generic-sections-refactor-memory_intersects-fix
+++ a/include/asm-generic/sections.h
@@ -97,7 +97,7 @@ static inline bool memory_contains(void
/**
* memory_intersects - checks if the region occupied by an object intersects
* with another memory region
- * @begin: virtual address of the beginning of the memory regien
+ * @begin: virtual address of the beginning of the memory region
* @end: virtual address of the end of the memory region
* @virt: virtual address of the memory object
* @size: size of the memory object
_
\
 
 \ /
  Last update: 2022-08-19 23:52    [W:0.061 / U:0.264 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site