Messages in this thread | | | Date | Sat, 20 Aug 2022 08:24:52 +0800 | Subject | Re: [V2][PATCH] asm-generic: sections: refactor memory_intersects | From | quanyang wang <> |
| |
Hi Andrew,
On 2022/8/20 05:51, Andrew Morton wrote: > 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?
Before the commit 1d7db834a027e ("dma-debug: use memory_intersects() directly") , memory_intersects is called only by printk_late_init:
printk_late_init -> init_section_intersects ->memory_intersects.
There are few places memory_intersects is called.
When the commit 1d7db834a027e ("dma-debug: use memory_intersects() directly") is merged and CONFIG_DMA_API_DEBUG is enabled,
DMA subsystem uses it to check illegal area and trigger the calltrace above.
Thanks,
Quanyang
> > > 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 > _ >
| |