linux-kernel - Re: helping with remapping vmem for dma

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]

Date:   Fri, 17 Jun 2022 18:22:35 +0100
From:   Robin Murphy <robin.murphy@....com>
To:     Frank Wunderlich <frank-w@...lic-files.de>,
        Christoph Hellwig <hch@....de>
Cc:     linux-kernel@...r.kernel.org, iommu@...ts.linux-foundation.org,
        Marek Szyprowski <m.szyprowski@...sung.com>
Subject: Re: helping with remapping vmem for dma

On 2022-06-17 17:17, Frank Wunderlich wrote:
> Am 15. Juni 2022 15:17:00 MESZ schrieb Christoph Hellwig <hch@....de>:
>> On Wed, Jun 15, 2022 at 02:15:33PM +0100, Robin Murphy wrote:
>>> Put simply, if you want to call dma_map_single() on a buffer, then that
>>> buffer needs to be allocated with kmalloc() (or technically alloc_pages(),
>>> but then dma_map_page() would make more sense when dealing with entire
>>> pages.
>>
>> Yes.  It sounds like the memory here comes from the dma coherent
>> allocator, in which case the code need to use the address returned
>> by that and not create another mapping.
> 
> Hi
> 
> traced it to buffer allocated as simple uint8-array [1]:
> 
> UINT_8 aucBuffer[sizeof(INIT_HIF_RX_HEADER_T) + sizeof(INIT_EVENT_CMD_RESULT)];

Ah, so it's trying to do DMA with a stack variable? CONFIG_DMA_API_DEBUG 
is your friend; it should have screamed about that specifically. 
Allocate this buffer properly to begin with, and free it again on the 
way out of the function (it's surely not worth having to make a 
temporary copy further down the callchain). The kmalloc flags can 
probably be regular GFP_KERNEL, unless this can be called from more 
restrictive contexts like an IRQ handler - the fact that it might be 
mapped for DMA later is essentially irrelevant in that respect.

Thanks,
Robin.

> 
> and then called as
> 
> nicRxWaitResponse(prAdapter,0,aucBuffer,sizeof(INIT_HIF_RX_HEADER_T) + sizeof(INIT_EVENT_CMD_RESULT),/* 4B + 4B */
> 					&u4RxPktLength)
> 
> this calls [2]:
> 
> WLAN_STATUS
> nicRxWaitResponse(IN P_ADAPTER_T prAdapter,
> 		  IN UINT_8 ucPortIdx, OUT PUINT_8 pucRspBuffer, IN UINT_32 u4MaxRespBufferLen, OUT PUINT_32 pu4Length)
> {
> ...
> HAL_PORT_RD(prAdapter,ucPortIdx == 0 ? MCR_WRDR0 : MCR_WRDR1, u4PktLen, pucRspBuffer, u4MaxRespBufferLen);
> }
> 
> 
> nicRxWaitResponse contains a do-while(true)-loop, but it looks like this is failing on first call (i see my debug before call of HAL_PORT_RD only once)...
> 
> do i need kmalloc before call of nicRxWaitResponse and if yes which flags are right here?
> https://www.kernel.org/doc/htmldocs/kernel-api/API-kmalloc.html
> 
> callstack is like this:
> 
> [  126.919569]  __dma_page_dev_to_cpu from kalDevPortRead+0x3a0/0x6b4 [wlan_gen2]
> [  126.928643]  kalDevPortRead [wlan_gen2] from nicRxWaitResponse+0x4c0/0x61c [wlan_gen2]
> [  126.939752]  nicRxWaitResponse [wlan_gen2] from wlanImageSectionDownloadStatus.part.0+0xd0/0x26c [wlan_gen2]
> [  126.952783]  wlanImageSectionDownloadStatus.part.0 [wlan_gen2] from wlanImageSectionDownload+0x168/0x36c [wlan_gen2]
> [  126.966511]  wlanImageSectionDownload [wlan_gen2] from wlanAdapterStart+0x674/0xf94 [wlan_gen2]
> [  126.978367]  wlanAdapterStart [wlan_gen2] from wlanProbe+0x318/0xbe8 [wlan_gen2]
> [  126.988951]  wlanProbe [wlan_gen2] from HifAhbProbe+0x54/0x88 [wlan_gen2]
> [  126.998905]  HifAhbProbe [wlan_gen2] from wmt_func_wifi_on+0x4c/0x150
> 
> regards Frank
> 
> [1] https://github.com/frank-w/BPI-R2-4.14/blob/5.18-main/drivers/misc/mediatek/connectivity/wlan/gen2/common/wlan_lib.c#L3046
> [2] https://github.com/frank-w/BPI-R2-4.14/blob/5.18-main/drivers/misc/mediatek/connectivity/wlan/gen2/nic/nic_rx.c#L3604