diff mbox

[dpdk-dev,v2] eal: Fix resource leak while secondary process maps pci devices

Message ID 1466044391-3210-3-git-send-email-mukawa@igel.co.jp (mailing list archive)
State Accepted, archived
Delegated to: Thomas Monjalon
Headers

Commit Message

Tetsuya Mukawa June 16, 2016, 2:33 a.m. UTC 
  This patch fixes resource leak of pci_uio_map_secondary().
If pci_map_resource() succeeds but mapped address is different from an
address primary process mapped, this should be error.
Then the addresses secondary process mapped should be freed.

Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
---
 lib/librte_eal/common/eal_common_pci_uio.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

Comments

David Marchand June 17, 2016, 12:28 p.m. UTC | #1 
On Thu, Jun 16, 2016 at 4:33 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
> This patch fixes resource leak of pci_uio_map_secondary().
> If pci_map_resource() succeeds but mapped address is different from an
> address primary process mapped, this should be error.
> Then the addresses secondary process mapped should be freed.
>
> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>

scripts/check-git-log.sh is not happy :

Wrong headline uppercase:
    eal: Fix resource leak while secondary process maps pci devices
Wrong headline lowercase:
    eal: Fix resource leak while secondary process maps pci devices
Headline too long:
    eal: Fix resource leak while secondary process maps pci devices
Missing 'Fixes' tag:
    eal: Fix resource leak while secondary process maps pci devices


checkpatch is not happy, but I think we can ignore it.

WARNING:LONG_LINE: line over 80 characters
#48: FILE: lib/librte_eal/common/eal_common_pci_uio.c:93:
+                            (size_t)uio_res->maps[j].size);


Anyways, looks good to me, Thomas, can you fix the commit logs of
those last 3 patches on eal ?
Thanks.
Tetsuya Mukawa June 20, 2016, 2:19 a.m. UTC | #2 
On 2016/06/17 21:28, David Marchand wrote:
> On Thu, Jun 16, 2016 at 4:33 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
>> This patch fixes resource leak of pci_uio_map_secondary().
>> If pci_map_resource() succeeds but mapped address is different from an
>> address primary process mapped, this should be error.
>> Then the addresses secondary process mapped should be freed.
>>
>> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> 
> scripts/check-git-log.sh is not happy :
> 
> Wrong headline uppercase:
>     eal: Fix resource leak while secondary process maps pci devices
> Wrong headline lowercase:
>     eal: Fix resource leak while secondary process maps pci devices
> Headline too long:
>     eal: Fix resource leak while secondary process maps pci devices
> Missing 'Fixes' tag:
>     eal: Fix resource leak while secondary process maps pci devices
> 
> 
> checkpatch is not happy, but I think we can ignore it.
> 
> WARNING:LONG_LINE: line over 80 characters
> #48: FILE: lib/librte_eal/common/eal_common_pci_uio.c:93:
> +                            (size_t)uio_res->maps[j].size);
> 
> 
> Anyways, looks good to me, Thomas, can you fix the commit logs of
> those last 3 patches on eal ?
> Thanks.
> 

Hi David,

I appreciate your checking.
Next time I will check check-git-log.sh before submitting.


Hi Thomas,

Could you please let me know if you need the fixed patches.

Best,
Tetsuya
Thomas Monjalon June 20, 2016, 8:50 a.m. UTC | #3 
2016-06-20 11:19, Tetsuya Mukawa:
> On 2016/06/17 21:28, David Marchand wrote:
> > On Thu, Jun 16, 2016 at 4:33 AM, Tetsuya Mukawa <mukawa@igel.co.jp> wrote:
> >> This patch fixes resource leak of pci_uio_map_secondary().
> >> If pci_map_resource() succeeds but mapped address is different from an
> >> address primary process mapped, this should be error.
> >> Then the addresses secondary process mapped should be freed.
> >>
> >> Signed-off-by: Tetsuya Mukawa <mukawa@igel.co.jp>
> > 
> > scripts/check-git-log.sh is not happy :
> > 
> > Wrong headline uppercase:
> >     eal: Fix resource leak while secondary process maps pci devices
> > Wrong headline lowercase:
> >     eal: Fix resource leak while secondary process maps pci devices
> > Headline too long:
> >     eal: Fix resource leak while secondary process maps pci devices
> > Missing 'Fixes' tag:
> >     eal: Fix resource leak while secondary process maps pci devices
> > 
> > 
> > checkpatch is not happy, but I think we can ignore it.
> > 
> > WARNING:LONG_LINE: line over 80 characters
> > #48: FILE: lib/librte_eal/common/eal_common_pci_uio.c:93:
> > +                            (size_t)uio_res->maps[j].size);
> > 
> > 
> > Anyways, looks good to me, Thomas, can you fix the commit logs of
> > those last 3 patches on eal ?
> > Thanks.
> > 
> 
> Hi David,
> 
> I appreciate your checking.
> Next time I will check check-git-log.sh before submitting.
> 
> 
> Hi Thomas,
> 
> Could you please let me know if you need the fixed patches.

Applied, thanks
diff mbox

Patch

diff --git a/lib/librte_eal/common/eal_common_pci_uio.c b/lib/librte_eal/common/eal_common_pci_uio.c
index 488b6dd..83314c3 100644
--- a/lib/librte_eal/common/eal_common_pci_uio.c
+++ b/lib/librte_eal/common/eal_common_pci_uio.c
@@ -53,7 +53,7 @@  EAL_REGISTER_TAILQ(rte_uio_tailq)
 static int
 pci_uio_map_secondary(struct rte_pci_device *dev)
 {
-	int fd, i;
+	int fd, i, j;
 	struct mapped_pci_resource *uio_res;
 	struct mapped_pci_res_list *uio_res_list =
 			RTE_TAILQ_CAST(rte_uio_tailq.head, mapped_pci_res_list);
@@ -85,6 +85,17 @@  pci_uio_map_secondary(struct rte_pci_device *dev)
 					"Cannot mmap device resource file %s to address: %p\n",
 					uio_res->maps[i].path,
 					uio_res->maps[i].addr);
+				if (mapaddr != MAP_FAILED) {
+					/* unmap addrs correctly mapped */
+					for (j = 0; j < i; j++)
+						pci_unmap_resource(
+							uio_res->maps[j].addr,
+							(size_t)uio_res->maps[j].size);
+
+					/* unmap addr wrongly mapped */
+					pci_unmap_resource(mapaddr,
+						(size_t)uio_res->maps[i].size);
+				}
 				return -1;
 			}
 		}