diff mbox series

net: vmxnet3: avoid accessing the data mapped to streaming DMA

Message ID 20200802131107.15857-1-baijiaju@tsinghua.edu.cn
State Rejected
Delegated to: David Miller
Headers show
Series net: vmxnet3: avoid accessing the data mapped to streaming DMA | expand

Commit Message

Jia-Ju Bai Aug. 2, 2020, 1:11 p.m. UTC
In vmxnet3_probe_device(), "adapter" is mapped to streaming DMA:
  adapter->adapter_pa = dma_map_single(..., adapter, ...);

Then "adapter" is accessed at many places in this function.

Theses accesses may cause data inconsistency between CPU cache and 
hardware.

To fix this problem, dma_map_single() is called after these accesses.

Signed-off-by: Jia-Ju Bai <baijiaju@tsinghua.edu.cn>
---
 drivers/net/vmxnet3/vmxnet3_drv.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

Comments

David Miller Aug. 3, 2020, 10:59 p.m. UTC | #1
From: Jia-Ju Bai <baijiaju@tsinghua.edu.cn>
Date: Sun,  2 Aug 2020 21:11:07 +0800

> In vmxnet3_probe_device(), "adapter" is mapped to streaming DMA:
>   adapter->adapter_pa = dma_map_single(..., adapter, ...);
> 
> Then "adapter" is accessed at many places in this function.
> 
> Theses accesses may cause data inconsistency between CPU cache and 
> hardware.
> 
> To fix this problem, dma_map_single() is called after these accesses.
> 
> Signed-off-by: Jia-Ju Bai <baijiaju@tsinghua.edu.cn>

'adapter' is accessed everywhere, in the entire driver, not just here
in the probe function.
Jia-Ju Bai Aug. 4, 2020, 2:17 a.m. UTC | #2
On 2020/8/4 6:59, David Miller wrote:
> From: Jia-Ju Bai <baijiaju@tsinghua.edu.cn>
> Date: Sun,  2 Aug 2020 21:11:07 +0800
>
>> In vmxnet3_probe_device(), "adapter" is mapped to streaming DMA:
>>    adapter->adapter_pa = dma_map_single(..., adapter, ...);
>>
>> Then "adapter" is accessed at many places in this function.
>>
>> Theses accesses may cause data inconsistency between CPU cache and
>> hardware.
>>
>> To fix this problem, dma_map_single() is called after these accesses.
>>
>> Signed-off-by: Jia-Ju Bai <baijiaju@tsinghua.edu.cn>
> 'adapter' is accessed everywhere, in the entire driver, not just here
> in the probe function.

Okay, replacing dma_map_single() with dma_alloc_coherent() may be better.
If you think this solution is okay, I can submit a new patch.


Best wishes,
Jia-Ju Bai
diff mbox series

Patch

diff --git a/drivers/net/vmxnet3/vmxnet3_drv.c b/drivers/net/vmxnet3/vmxnet3_drv.c
index ca395f9679d0..96a4c28ba429 100644
--- a/drivers/net/vmxnet3/vmxnet3_drv.c
+++ b/drivers/net/vmxnet3/vmxnet3_drv.c
@@ -3445,14 +3445,6 @@  vmxnet3_probe_device(struct pci_dev *pdev,
 	}
 
 	spin_lock_init(&adapter->cmd_lock);
-	adapter->adapter_pa = dma_map_single(&adapter->pdev->dev, adapter,
-					     sizeof(struct vmxnet3_adapter),
-					     PCI_DMA_TODEVICE);
-	if (dma_mapping_error(&adapter->pdev->dev, adapter->adapter_pa)) {
-		dev_err(&pdev->dev, "Failed to map dma\n");
-		err = -EFAULT;
-		goto err_set_mask;
-	}
 	adapter->shared = dma_alloc_coherent(
 				&adapter->pdev->dev,
 				sizeof(struct Vmxnet3_DriverShared),
@@ -3628,6 +3620,16 @@  vmxnet3_probe_device(struct pci_dev *pdev,
 	}
 
 	vmxnet3_check_link(adapter, false);
+
+	adapter->adapter_pa = dma_map_single(&adapter->pdev->dev, adapter,
+					     sizeof(struct vmxnet3_adapter),
+					     PCI_DMA_TODEVICE);
+	if (dma_mapping_error(&adapter->pdev->dev, adapter->adapter_pa)) {
+		dev_err(&pdev->dev, "Failed to map dma\n");
+		err = -EFAULT;
+		goto err_register;
+	}
+
 	return 0;
 
 err_register:
@@ -3655,8 +3657,6 @@  vmxnet3_probe_device(struct pci_dev *pdev,
 			  sizeof(struct Vmxnet3_DriverShared),
 			  adapter->shared, adapter->shared_pa);
 err_alloc_shared:
-	dma_unmap_single(&adapter->pdev->dev, adapter->adapter_pa,
-			 sizeof(struct vmxnet3_adapter), PCI_DMA_TODEVICE);
 err_set_mask:
 	free_netdev(netdev);
 	return err;