| Message ID | 20191010162653.141303-1-bjorn.andersson@linaro.org (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Kalle Valo |
| Headers | show |
| Series | ath10k: Correct error check of dma_map_single() | expand |
Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > The return value of dma_map_single() should be checked for errors using > dma_mapping_error(), rather than testing for NULL. Correct this. > > Fixes: 1807da49733e ("ath10k: wmi: add management tx by reference support over wmi") > Cc: stable@vger.kernel.org > Reported-by: Niklas Cassel <niklas.cassel@linaro.org> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> Did this fix any real bug? Or is this just something found during code review?
On Fri 11 Oct 04:57 PDT 2019, Kalle Valo wrote: > Bjorn Andersson <bjorn.andersson@linaro.org> wrote: > > > The return value of dma_map_single() should be checked for errors using > > dma_mapping_error(), rather than testing for NULL. Correct this. > > > > Fixes: 1807da49733e ("ath10k: wmi: add management tx by reference support over wmi") > > Cc: stable@vger.kernel.org > > Reported-by: Niklas Cassel <niklas.cassel@linaro.org> > > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> > > Did this fix any real bug? Or is this just something found during code review? > CONFIG_DMA_API_DEBUG screamed at us for calling dma_unmap_single() without ever having called dma_mapping_error() on the return value. But Govind just pointed out to me that I hastily missed the fact that this code path leaks the dequeued skb. So I'll respin the patch to fix both issues at once. Regards, Bjorn > -- > https://patchwork.kernel.org/patch/11183923/ > > https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches > > > _______________________________________________ > ath10k mailing list > ath10k@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/ath10k
Bjorn Andersson <bjorn.andersson@linaro.org> writes: > On Fri 11 Oct 04:57 PDT 2019, Kalle Valo wrote: > >> Bjorn Andersson <bjorn.andersson@linaro.org> wrote: >> >> > The return value of dma_map_single() should be checked for errors using >> > dma_mapping_error(), rather than testing for NULL. Correct this. >> > >> > Fixes: 1807da49733e ("ath10k: wmi: add management tx by reference >> > support over wmi") >> > Cc: stable@vger.kernel.org >> > Reported-by: Niklas Cassel <niklas.cassel@linaro.org> >> > Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> >> >> Did this fix any real bug? Or is this just something found during code review? >> > > CONFIG_DMA_API_DEBUG screamed at us for calling dma_unmap_single() > without ever having called dma_mapping_error() on the return value. Ok, I'll add something about to the commit log in v2 so that the background is also documented.
diff --git a/drivers/net/wireless/ath/ath10k/mac.c b/drivers/net/wireless/ath/ath10k/mac.c index 3d2c8fcba952..a01868938692 100644 --- a/drivers/net/wireless/ath/ath10k/mac.c +++ b/drivers/net/wireless/ath/ath10k/mac.c @@ -3904,7 +3904,7 @@ void ath10k_mgmt_over_wmi_tx_work(struct work_struct *work) ar->running_fw->fw_file.fw_features)) { paddr = dma_map_single(ar->dev, skb->data, skb->len, DMA_TO_DEVICE); - if (!paddr) + if (dma_mapping_error(ar->dev, paddr)) continue; ret = ath10k_wmi_mgmt_tx_send(ar, skb, paddr); if (ret) {
The return value of dma_map_single() should be checked for errors using dma_mapping_error(), rather than testing for NULL. Correct this. Fixes: 1807da49733e ("ath10k: wmi: add management tx by reference support over wmi") Cc: stable@vger.kernel.org Reported-by: Niklas Cassel <niklas.cassel@linaro.org> Signed-off-by: Bjorn Andersson <bjorn.andersson@linaro.org> --- drivers/net/wireless/ath/ath10k/mac.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)