Bug 189041 - Fuction qed_ll2_start_xmit() does not set error code when the call to dma_mapping_error() fails
Summary: Fuction qed_ll2_start_xmit() does not set error code when the call to dma_map...
Status: RESOLVED CODE_FIX
Alias: None
Product: Drivers
Classification: Unclassified
Component: Network (show other bugs)
Hardware: All Linux
: P1 normal
Assignee: drivers_network@kernel-bugs.osdl.org
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-11-25 11:19 UTC by bianpan
Modified: 2017-05-12 00:28 UTC (History)
0 users

See Also:
Kernel Version: linux-4.9-rc6
Subsystem:
Regression: No
Bisected commit-id:


Attachments
The patch fixes the bug (1.14 KB, patch)
2017-05-12 00:27 UTC, bianpan
Details | Diff

Description bianpan 2016-11-25 11:19:17 UTC
In the function qed_ll2_start_xmit() defined in file drivers/net/ethernet/qlogic/qed/qed_ll2.c, when the call to dma_mapping_error() (at line 1729) returns an unexpected value, the control flow jumps to label "err", and returns variable rc. Because variable rc is checked at line 1719, the value of rc must be 0 here. As a result, function qed_ll2_start_xmit() returns 0 (indicates success) even if the call to dma_mapping_error() fails. Though this error may occur rarely, I think it may be better to set an correct error code (e.g. -ENOMEM) on the failure. Codes related to this bug are summarised as follows.

1678 static int qed_ll2_start_xmit(struct qed_dev *cdev, struct sk_buff *skb)
1679 {
1680     const skb_frag_t *frag;
1681     int rc = -EINVAL, i;
         ...
1719     if (rc)
1720         goto err;
1722     for (i = 0; i < skb_shinfo(skb)->nr_frags; i++) {
1723         frag = &skb_shinfo(skb)->frags[i];
1724         if (!cdev->ll2->frags_mapped) {
1725             mapping = skb_frag_dma_map(&cdev->pdev->dev, frag, 0,
1726                            skb_frag_size(frag),
1727                            DMA_TO_DEVICE);
1728 
1729             if (unlikely(dma_mapping_error(&cdev->pdev->dev,
1730                                mapping))) {
1731                 DP_NOTICE(cdev,
1732                       "Unable to map frag - dropping packet\n");
                     // Bug: the value of rc is 0, insert "rc = -ENOMEM" ?
1733                 goto err;
1734             }
1735         } else {
1736             mapping = page_to_phys(skb_frag_page(frag)) |
1737                 frag->page_offset;
1738         }
1739 
1740         rc = qed_ll2_set_fragment_of_tx_packet(QED_LEADING_HWFN(cdev),
1741                                cdev->ll2->handle,
1742                                mapping,
1743                                skb_frag_size(frag));
1744 
1745         /* if failed not much to do here, partial packet has been posted
1746          * we can't free memory, will need to wait for completion.
1747          */
1748         if (rc)
1749             goto err2;
1750     }
1751 
1752     return 0;
1753 
1754 err:
1755     dma_unmap_single(&cdev->pdev->dev, mapping, skb->len, DMA_TO_DEVICE);
1756 
1757 err2:
1758     return rc;
1759 }

Thanks very much!
Comment 1 bianpan 2017-05-12 00:27:56 UTC
Created attachment 256453 [details]
The patch fixes the bug

The patch has been merged into the latest version of the Linux kernel. So I will close the bug.

Note You need to log in before you can comment on or make changes to this bug.