Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1315471 > unrolled thread

[PATCH v2] mptlan: add checks for dma mapping errors

Back to article view | Back to linux.kernel

Contents

  [PATCH v2] mptlan: add checks for dma mapping errors Alexey Khoroshilov <khoroshilov@ispras.ru> - 2016-01-23 01:50 +0100
    Re: [PATCH v2] mptlan: add checks for dma mapping errors Tomas Henzl <thenzl@redhat.com> - 2016-01-25 16:40 +0100
      Re: [PATCH v2] mptlan: add checks for dma mapping errors Alexey Khoroshilov <khoroshilov@ispras.ru> - 2016-01-25 18:10 +0100
        [PATCH v3] mptlan: add checks for dma mapping errors Alexey Khoroshilov <khoroshilov@ispras.ru> - 2016-01-25 19:10 +0100
          Re: [PATCH v3] mptlan: add checks for dma mapping errors Tomas Henzl <thenzl@redhat.com> - 2016-01-26 17:40 +0100
            Re: [PATCH v3] mptlan: add checks for dma mapping errors "Martin K. Petersen" <martin.petersen@oracle.com> - 2016-01-27 03:30 +0100
              RE: [PATCH v3] mptlan: add checks for dma mapping errors Sathya Prakash <sathya.prakash@avagotech.com> - 2016-01-27 06:50 +0100
                Re: [PATCH v3] mptlan: add checks for dma mapping errors Tomas Henzl <thenzl@redhat.com> - 2016-01-27 17:20 +0100
                  Re: [PATCH v3] mptlan: add checks for dma mapping errors James Bottomley <James.Bottomley@HansenPartnership.com> - 2016-01-27 17:30 +0100

#1315471 — [PATCH v2] mptlan: add checks for dma mapping errors

mpt_lan_sdu_send() and mpt_lan_post_receive_buckets() do not check
if mapping dma memory succeed.
The patch adds the checks and failure handling.

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/message/fusion/mptlan.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/drivers/message/fusion/mptlan.c b/drivers/message/fusion/mptlan.c
index cbe96072a6cc..3b6c8a755713 100644
--- a/drivers/message/fusion/mptlan.c
+++ b/drivers/message/fusion/mptlan.c
@@ -734,6 +734,12 @@ mpt_lan_sdu_send (struct sk_buff *skb, struct net_device *dev)
 
         dma = pci_map_single(mpt_dev->pcidev, skb->data, skb->len,
 			     PCI_DMA_TODEVICE);
+	if (pci_dma_mapping_error(mpt_dev->pcidev, dma)) {
+		netif_stop_queue(dev);
+
+		printk (KERN_ERR "%s: dma mapping failed\n", __func__);
+		return NETDEV_TX_BUSY;
+	}
 
 	priv->SendCtl[ctx].skb = skb;
 	priv->SendCtl[ctx].dma = dma;
@@ -1232,6 +1238,14 @@ mpt_lan_post_receive_buckets(struct mpt_lan_priv *priv)
 
 				dma = pci_map_single(mpt_dev->pcidev, skb->data,
 						     len, PCI_DMA_FROMDEVICE);
+				if (pci_dma_mapping_error(mpt_dev->pcidev, dma)) {
+					printk (KERN_WARNING
+						MYNAM "/%s: dma mapping failed\n",
+						__func__);
+					priv->mpt_rxfidx[++priv->mpt_rxfidx_tail] = ctx;
+					spin_unlock_irqrestore(&priv->rxfidx_lock, flags);
+					break;
+				}
 
 				priv->RcvCtl[ctx].skb = skb;
 				priv->RcvCtl[ctx].dma = dma;
-- 
1.9.1

[toc] | [next] | [standalone]

#1316825

On 23.1.2016 01:41, Alexey Khoroshilov wrote:
> mpt_lan_sdu_send() and mpt_lan_post_receive_buckets() do not check
> if mapping dma memory succeed.
> The patch adds the checks and failure handling.
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> ---
>  drivers/message/fusion/mptlan.c | 14 ++++++++++++++
>  1 file changed, 14 insertions(+)
>
> diff --git a/drivers/message/fusion/mptlan.c b/drivers/message/fusion/mptlan.c
> index cbe96072a6cc..3b6c8a755713 100644
> --- a/drivers/message/fusion/mptlan.c
> +++ b/drivers/message/fusion/mptlan.c
> @@ -734,6 +734,12 @@ mpt_lan_sdu_send (struct sk_buff *skb, struct net_device *dev)
>  
>          dma = pci_map_single(mpt_dev->pcidev, skb->data, skb->len,
>  			     PCI_DMA_TODEVICE);
> +	if (pci_dma_mapping_error(mpt_dev->pcidev, dma)) {
> +		netif_stop_queue(dev);

Hi Alexey,
isn't a mpt_put_msg_frame needed here ?
and a similar de-alloc in next chunk too?
-tms

> +
> +		printk (KERN_ERR "%s: dma mapping failed\n", __func__);
> +		return NETDEV_TX_BUSY;
> +	}
>  
>  	priv->SendCtl[ctx].skb = skb;
>  	priv->SendCtl[ctx].dma = dma;
> @@ -1232,6 +1238,14 @@ mpt_lan_post_receive_buckets(struct mpt_lan_priv *priv)
>  
>  				dma = pci_map_single(mpt_dev->pcidev, skb->data,
>  						     len, PCI_DMA_FROMDEVICE);
> +				if (pci_dma_mapping_error(mpt_dev->pcidev, dma)) {
> +					printk (KERN_WARNING
> +						MYNAM "/%s: dma mapping failed\n",
> +						__func__);
> +					priv->mpt_rxfidx[++priv->mpt_rxfidx_tail] = ctx;
> +					spin_unlock_irqrestore(&priv->rxfidx_lock, flags);
> +					break;
> +				}
>  
>  				priv->RcvCtl[ctx].skb = skb;
>  				priv->RcvCtl[ctx].dma = dma;

[toc] | [prev] | [next] | [standalone]

#1317064

On 25.01.2016 16:36, Tomas Henzl wrote:
> On 23.1.2016 01:41, Alexey Khoroshilov wrote:
>> mpt_lan_sdu_send() and mpt_lan_post_receive_buckets() do not check
>> if mapping dma memory succeed.
>> The patch adds the checks and failure handling.
>>
>> Found by Linux Driver Verification project (linuxtesting.org).
>>
>> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
>> ---
>>  drivers/message/fusion/mptlan.c | 14 ++++++++++++++
>>  1 file changed, 14 insertions(+)
>>
>> diff --git a/drivers/message/fusion/mptlan.c b/drivers/message/fusion/mptlan.c
>> index cbe96072a6cc..3b6c8a755713 100644
>> --- a/drivers/message/fusion/mptlan.c
>> +++ b/drivers/message/fusion/mptlan.c
>> @@ -734,6 +734,12 @@ mpt_lan_sdu_send (struct sk_buff *skb, struct net_device *dev)
>>  
>>          dma = pci_map_single(mpt_dev->pcidev, skb->data, skb->len,
>>  			     PCI_DMA_TODEVICE);
>> +	if (pci_dma_mapping_error(mpt_dev->pcidev, dma)) {
>> +		netif_stop_queue(dev);
> 
> Hi Alexey,
> isn't a mpt_put_msg_frame needed here ?
> and a similar de-alloc in next chunk too?
> -tms

Hi Tomas,

You are right, thank you!
I'll resend the patch.

--
Alexey

[toc] | [prev] | [next] | [standalone]

#1317139 — [PATCH v3] mptlan: add checks for dma mapping errors

mpt_lan_sdu_send() and mpt_lan_post_receive_buckets() do not check
if mapping dma memory succeed.
The patch adds the checks and failure handling.

v3: Fix resource deallocation (reported by Tomas Henzl).

Found by Linux Driver Verification project (linuxtesting.org).

Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
---
 drivers/message/fusion/mptlan.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/drivers/message/fusion/mptlan.c b/drivers/message/fusion/mptlan.c
index cbe96072a6cc..e9b83fc7be35 100644
--- a/drivers/message/fusion/mptlan.c
+++ b/drivers/message/fusion/mptlan.c
@@ -734,6 +734,13 @@ mpt_lan_sdu_send (struct sk_buff *skb, struct net_device *dev)
 
         dma = pci_map_single(mpt_dev->pcidev, skb->data, skb->len,
 			     PCI_DMA_TODEVICE);
+	if (pci_dma_mapping_error(mpt_dev->pcidev, dma)) {
+		mpt_put_msg_frame(LanCtx, mpt_dev, mf);
+		netif_stop_queue(dev);
+
+		printk (KERN_ERR "%s: dma mapping failed\n", __func__);
+		return NETDEV_TX_BUSY;
+	}
 
 	priv->SendCtl[ctx].skb = skb;
 	priv->SendCtl[ctx].dma = dma;
@@ -1232,6 +1239,15 @@ mpt_lan_post_receive_buckets(struct mpt_lan_priv *priv)
 
 				dma = pci_map_single(mpt_dev->pcidev, skb->data,
 						     len, PCI_DMA_FROMDEVICE);
+				if (pci_dma_mapping_error(mpt_dev->pcidev, dma)) {
+					printk (KERN_WARNING
+						MYNAM "/%s: dma mapping failed\n",
+						__func__);
+					dev_kfree_skb(skb);
+					priv->mpt_rxfidx[++priv->mpt_rxfidx_tail] = ctx;
+					spin_unlock_irqrestore(&priv->rxfidx_lock, flags);
+					break;
+				}
 
 				priv->RcvCtl[ctx].skb = skb;
 				priv->RcvCtl[ctx].dma = dma;
-- 
1.9.1

[toc] | [prev] | [next] | [standalone]

#1318121 — Re: [PATCH v3] mptlan: add checks for dma mapping errors

On 25.1.2016 19:02, Alexey Khoroshilov wrote:
> mpt_lan_sdu_send() and mpt_lan_post_receive_buckets() do not check
> if mapping dma memory succeed.
> The patch adds the checks and failure handling.
>
> v3: Fix resource deallocation (reported by Tomas Henzl).
>
> Found by Linux Driver Verification project (linuxtesting.org).
>
> Signed-off-by: Alexey Khoroshilov <khoroshilov@ispras.ru>
> ---
>  drivers/message/fusion/mptlan.c | 16 ++++++++++++++++
>  1 file changed, 16 insertions(+)
>
> diff --git a/drivers/message/fusion/mptlan.c b/drivers/message/fusion/mptlan.c
> index cbe96072a6cc..e9b83fc7be35 100644
> --- a/drivers/message/fusion/mptlan.c
> +++ b/drivers/message/fusion/mptlan.c
> @@ -734,6 +734,13 @@ mpt_lan_sdu_send (struct sk_buff *skb, struct net_device *dev)
>  
>          dma = pci_map_single(mpt_dev->pcidev, skb->data, skb->len,
>  			     PCI_DMA_TODEVICE);
> +	if (pci_dma_mapping_error(mpt_dev->pcidev, dma)) {
> +		mpt_put_msg_frame(LanCtx, mpt_dev, mf);

I think I was wrong here, the 'mpt_put_msg_frame' is not the correct function
for freeing the mpt request frame, this one actually talks to the hw.

Other than that - previous patch for this driver came in in 2010
 - six years ago and the driver seems unmaintained now.
I'm not sure if we should fix hw we can't test and when there is not
an user bug report. This example nicely shows how easy it is to add new bugs
even when a fix looks trivial.
-tm

> +		netif_stop_queue(dev);
> +
> +		printk (KERN_ERR "%s: dma mapping failed\n", __func__);
> +		return NETDEV_TX_BUSY;
> +	}
>  
>  	priv->SendCtl[ctx].skb = skb;
>  	priv->SendCtl[ctx].dma = dma;
> @@ -1232,6 +1239,15 @@ mpt_lan_post_receive_buckets(struct mpt_lan_priv *priv)
>  
>  				dma = pci_map_single(mpt_dev->pcidev, skb->data,
>  						     len, PCI_DMA_FROMDEVICE);
> +				if (pci_dma_mapping_error(mpt_dev->pcidev, dma)) {
> +					printk (KERN_WARNING
> +						MYNAM "/%s: dma mapping failed\n",
> +						__func__);
> +					dev_kfree_skb(skb);
> +					priv->mpt_rxfidx[++priv->mpt_rxfidx_tail] = ctx;
> +					spin_unlock_irqrestore(&priv->rxfidx_lock, flags);
> +					break;
> +				}
>  
>  				priv->RcvCtl[ctx].skb = skb;
>  				priv->RcvCtl[ctx].dma = dma;

[toc] | [prev] | [next] | [standalone]

#1318560 — Re: [PATCH v3] mptlan: add checks for dma mapping errors

>>>>> "Tomas" == Tomas Henzl <thenzl@redhat.com> writes:

Tomas> Other than that - previous patch for this driver came in in 2010
Tomas> - six years ago and the driver seems unmaintained now.  I'm not
Tomas> sure if we should fix hw we can't test and when there is not an
Tomas> user bug report. This example nicely shows how easy it is to add
Tomas> new bugs even when a fix looks trivial.

Yeah, I'm inclined to leave it as is.

If somebody provides a Tested-by: I'll reconsider.

-- 
Martin K. Petersen	Oracle Linux Engineering

[toc] | [prev] | [next] | [standalone]

#1318673 — RE: [PATCH v3] mptlan: add checks for dma mapping errors

There is no fusion based network card and resources exists today in
Avago(LSI) to test this patch so we prefer to leave it as is. We would
like to prevent any new changes on MPT (FC/SCSI/SAS/LAN) drivers as we
don't have support for those cards anymore,  is there a way we could
remove those drivers from newer kernels or mark them as unmaintained?.

Thanks
Sathya

-----Original Message-----
From: mpt-fusionlinux.pdl@avagotech.com
[mailto:mpt-fusionlinux.pdl@avagotech.com] On Behalf Of Martin K. Petersen
Sent: Tuesday, January 26, 2016 7:23 PM
To: Tomas Henzl
Cc: Alexey Khoroshilov; Sreekanth Reddy;
MPT-FusionLinux.pdl@avagotech.com; linux-scsi@vger.kernel.org;
linux-kernel@vger.kernel.org; ldv-project@linuxtesting.org
Subject: Re: [PATCH v3] mptlan: add checks for dma mapping errors

>>>>> "Tomas" == Tomas Henzl <thenzl@redhat.com> writes:

Tomas> Other than that - previous patch for this driver came in in 2010
Tomas> - six years ago and the driver seems unmaintained now.  I'm not
Tomas> sure if we should fix hw we can't test and when there is not an
Tomas> user bug report. This example nicely shows how easy it is to add
Tomas> new bugs even when a fix looks trivial.

Yeah, I'm inclined to leave it as is.

If somebody provides a Tested-by: I'll reconsider.

-- 
Martin K. Petersen	Oracle Linux Engineering

[toc] | [prev] | [next] | [standalone]

#1319118 — Re: [PATCH v3] mptlan: add checks for dma mapping errors

On 27.1.2016 06:44, Sathya Prakash wrote:
> There is no fusion based network card and resources exists today in
> Avago(LSI) to test this patch so we prefer to leave it as is. We would
> like to prevent any new changes on MPT (FC/SCSI/SAS/LAN) drivers as we
> don't have support for those cards anymore,  is there a way we could
> remove those drivers from newer kernels or mark them as unmaintained?.

There still are users of some of those drivers (mptsas for example)
in certain distributions, so even if in fact they aren't
directly maintained, we should keep them in mainline.

Thanks,
Tomas

>
> Thanks
> Sathya
>
> -----Original Message-----
> From: mpt-fusionlinux.pdl@avagotech.com
> [mailto:mpt-fusionlinux.pdl@avagotech.com] On Behalf Of Martin K. Petersen
> Sent: Tuesday, January 26, 2016 7:23 PM
> To: Tomas Henzl
> Cc: Alexey Khoroshilov; Sreekanth Reddy;
> MPT-FusionLinux.pdl@avagotech.com; linux-scsi@vger.kernel.org;
> linux-kernel@vger.kernel.org; ldv-project@linuxtesting.org
> Subject: Re: [PATCH v3] mptlan: add checks for dma mapping errors
>
>>>>>> "Tomas" == Tomas Henzl <thenzl@redhat.com> writes:
> Tomas> Other than that - previous patch for this driver came in in 2010
> Tomas> - six years ago and the driver seems unmaintained now.  I'm not
> Tomas> sure if we should fix hw we can't test and when there is not an
> Tomas> user bug report. This example nicely shows how easy it is to add
> Tomas> new bugs even when a fix looks trivial.
>
> Yeah, I'm inclined to leave it as is.
>
> If somebody provides a Tested-by: I'll reconsider.
>

[toc] | [prev] | [next] | [standalone]

#1319135 — Re: [PATCH v3] mptlan: add checks for dma mapping errors

On Wed, 2016-01-27 at 17:14 +0100, Tomas Henzl wrote:
> On 27.1.2016 06:44, Sathya Prakash wrote:
> > There is no fusion based network card and resources exists today in
> > Avago(LSI) to test this patch so we prefer to leave it as is. We
> > would
> > like to prevent any new changes on MPT (FC/SCSI/SAS/LAN) drivers as
> > we
> > don't have support for those cards anymore,  is there a way we
> > could
> > remove those drivers from newer kernels or mark them as
> > unmaintained?.
> 
> There still are users of some of those drivers (mptsas for example)
> in certain distributions, so even if in fact they aren't
> directly maintained, we should keep them in mainline.

Agreed: the last gen PA-RISC has a mptspi controller ... they'd get a
bit annoyed if we remove it because they wouldn't be able to update
their build machines to newer kernels.

James

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web