-----Original Message----- From: Arend van Spriel [mailto:arend.vanspriel@xxxxxxxxxxxx] Sent: Thursday, December 22, 2022 7:00 PM To: shaozhengchao <shaozhengchao@xxxxxxxxxx>; Kalle Valo <kvalo@xxxxxxxxxx>; Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> Cc: netdev@xxxxxxxxxxxxxxx; linux-wireless@xxxxxxxxxxxxxxx; brcm80211-dev-list.pdl@xxxxxxxxxxxx; SHA-cyfmac-dev-list@xxxxxxxxxxxx; davem@xxxxxxxxxxxxx; edumazet@xxxxxxxxxx; kuba@xxxxxxxxxx; pabeni@xxxxxxxxxx; aspriel@xxxxxxxxx; franky.lin@xxxxxxxxxxxx; hante.meuleman@xxxxxxxxxxxx; wright.feng@xxxxxxxxxxx; chi-hsien.lin@xxxxxxxxxxx; a.fatoum@xxxxxxxxxxxxxx; alsi@xxxxxxxxxxxxxxx; pieterpg@xxxxxxxxxxxx; dekim@xxxxxxxxxxxx; linville@xxxxxxxxxxxxx; weiyongjun (A) <weiyongjun1@xxxxxxxxxx>; yuehaibing <yuehaibing@xxxxxxxxxx> Subject: Re: [PATCH] wifi: brcmfmac: unmap dma buffer in brcmf_msgbuf_alloc_pktid() On 12/22/2022 9:52 AM, shaozhengchao wrote: > > > On 2022/12/22 16:46, Kalle Valo wrote: >> Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> writes: >> >>> On 2022-12-21 18:33:06 [+0000], Kalle Valo wrote: >>>> Zhengchao Shao <shaozhengchao@xxxxxxxxxx> wrote: >>>> >>>>> After the DMA buffer is mapped to a physical address, address is >>>>> stored >>>>> in pktids in brcmf_msgbuf_alloc_pktid(). Then, pktids is parsed in >>>>> brcmf_msgbuf_get_pktid()/brcmf_msgbuf_release_array() to obtain >>>>> physaddr >>>>> and later unmap the DMA buffer. But when count is always equal to >>>>> pktids->array_size, physaddr isn't stored in pktids and the DMA buffer >>>>> will not be unmapped anyway. >>>>> >>>>> Fixes: 9a1bb60250d2 ("brcmfmac: Adding msgbuf protocol.") >>>>> Signed-off-by: Zhengchao Shao <shaozhengchao@xxxxxxxxxx> >>>> >>>> Can someone review this? >>> >>> After looking at the code, that skb is mapped but not inserted into the >>> ringbuffer in this condition. The function returns with an error and the >>> caller will free that skb (or add to a list for later). Either way the >>> skb remains mapped which is wrong. The unmap here is the right thing to >>> do. >>> >>> Reviewed-by: Sebastian Andrzej Siewior <bigeasy@xxxxxxxxxxxxx> >> >> Thanks for the review, very much appreciated. >> > > Thank you very much. >Good catch. Has this path been observed or is this found by inspecting >the code? Just curious. >Regards, >Arend Hi Arend： I review code and find the bug. Zhengchao Shao
