Re: [PATCH] wcn36xx: check dma_mapping_error()

[Bjorn Andersson <bjorn.andersson@linaro.org>]

On Sun 02 Jul 06:46 PDT 2017, Rob Clark wrote:

> Fixes splat:
> 
>   wcn36xx a204000.wcnss:smd-edge:wcnss:wifi: DMA-API: device driver failed to check map error[device address=0x00000000b45ba000] [size=3872 bytes] [mapped as single]
>   ------------[ cut here ]------------
>   WARNING: CPU: 0 PID: 0 at ../lib/dma-debug.c:1167 check_unmap+0x474/0x8d0
>   Modules linked in: bnep(E) arc4(E) wcn36xx(E) mac80211(E) btqcomsmd(E) btqca(E) bluetooth(E) cfg80211(E) ecdh_generic(E) rfkill(E) vfat(E) fat(E) wcnss_ctrl qcom_wcnss_pil(E) mdt_loader(E) qcom_common(E) remoteproc(E) crc32_ce(E) virtio_ring(E) snd_soc_lpass_apq8016(E) snd_soc_lpass_cpu(E) virtio(E) snd_soc_lpass_platform(E) leds_gpio(E) snd_soc_hdmi_codec(E) snd_soc_apq8016_sbc(E) snd_soc_msm8916_digital(E) snd_soc_core(E) qcom_spmi_temp_alarm(E) ac97_bus(E) snd_pcm_dmaengine(E) snd_seq(E) snd_seq_device(E) snd_pcm(E) spi_qup(E) nvmem_qfprom(E) snd_timer(E) snd(E) soundcore(E) msm_rng(E) qcom_tsens(E) nvmem_core(E) uas(E) usb_storage(E) dm9601(E) cdc_ether(E) usbnet(E) mii(E) mmc_block(E) sdhci_msm(E) sdhci_pltfm(E) qcom_spmi_vadc(E) qcom_vadc_common(PE) clk_smd_rpm(E) industrialio(E)
 
>    qcom_smd_regulator(E) pinctrl_spmi_mpp(E) pinctrl_spmi_gpio(E) rtc_pm8xxx(E) adv7511(E) smd_rpm(E) qcom_spmi_pmic(E) regmap_spmi(E) phy_msm_usb(E) usb3503(E) extcon_usb_gpio(E) ci_hdrc_msm(E) ci_hdrc(E) qcom_hwspinlock(E) udc_core(E) extcon_core(E) ehci_msm(E) i2c_qup(E) sdhci(E) msm(E) mmc_core(E) drm_kms_helper(E) syscopyarea(E) sysfillrect(E) sysimgblt(E) fb_sys_fops(E) spmi_pmic_arb(E) drm(E) spmi(E) qcom_smd(E) rpmsg_core smsm(E) gpio_keys(E) smp2p(E) smem(E) hwspinlock_core(E) sunrpc(E) scsi_transport_iscsi(E)
>   CPU: 0 PID: 0 Comm: swapper/0 Tainted: P            E   4.12.0-rc7+ #1476
>   Hardware name: qualcomm dragonboard410c/dragonboard410c, BIOS 2017.07-rc1-00234-g22fa70a-dirty 06/26/2017
>   task: ffff000009049780 task.stack: ffff000009030000
>   PC is at check_unmap+0x474/0x8d0
>   LR is at check_unmap+0x474/0x8d0
>   ...
>   Mapped at:
>    dma_entry_alloc+0x68/0xa8
>    debug_dma_map_page+0x94/0x148
>    wcn36xx_dxe_fill_skb.isra.1+0xbc/0xf8 [wcn36xx]
>    wcn36xx_dxe_init+0x244/0x398 [wcn36xx]
>    wcn36xx_start+0xf4/0x298 [wcn36xx]
> 
> Signed-off-by: Rob Clark <robdclark@gmail.com>
> ---
>  drivers/net/wireless/ath/wcn36xx/dxe.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/drivers/net/wireless/ath/wcn36xx/dxe.c b/drivers/net/wireless/ath/wcn36xx/dxe.c
> index 87dfdaf..8e261b4 100644
> --- a/drivers/net/wireless/ath/wcn36xx/dxe.c
> +++ b/drivers/net/wireless/ath/wcn36xx/dxe.c
> @@ -289,6 +289,10 @@ static int wcn36xx_dxe_fill_skb(struct device *dev, struct wcn36xx_dxe_ctl *ctl)
>  					 skb_tail_pointer(skb),
>  					 WCN36XX_PKT_SIZE,
>  					 DMA_FROM_DEVICE);
> +	if (dma_mapping_error(dev, dxe->dst_addr_l)) {

Seems like it might be a bad idea to clobber dxe->dst_addr_l, so we
should probably check the result locally before doing the assignment.

> +		dev_err(dev, "unable to map skb\n");

You need to free the skb.

		kfree_skb(skb);

> +		return -ENOMEM;
> +	}
>  	ctl->skb = skb;
>  

Can you please resend this to Kalle Valo <kvalo@qca.qualcomm.com>
and Cc linux-wireless@vger.kernel.org

Regards,
Bjorn