linux-kernel - Re: [V2][PATCH] asm-generic: sections: refactor memory

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <a22358ce-8eb3-cd34-47e1-363ce37e0ae7@windriver.com>
Date:   Sat, 20 Aug 2022 08:24:52 +0800
From:   quanyang wang <quanyang.wang@...driver.com>
To:     Andrew Morton <akpm@...ux-foundation.org>
Cc:     Arnd Bergmann <arnd@...db.de>, Thierry Reding <treding@...dia.com>,
        Ard Biesheuvel <ardb@...nel.org>, linux-arch@...r.kernel.org,
        linux-kernel@...r.kernel.org
Subject: Re: [V2][PATCH] asm-generic: sections: refactor memory_intersects

Hi Andrew,

On 2022/8/20 05:51, Andrew Morton wrote:
> On Fri, 19 Aug 2022 16:11:45 +0800 quanyang.wang@...driver.com wrote:
>
>> From: Quanyang Wang <quanyang.wang@...driver.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
> _
>