Project

General

Profile

Actions

Bug #1788

closed

af-packet coverity warning

Added by Victor Julien almost 8 years ago. Updated almost 6 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

** CID 1362014:    (RESOURCE_LEAK)
/src/source-af-packet.c: 1799 in AFPSetupRing()
/src/source-af-packet.c: 1803 in AFPSetupRing()

________________________________________________________________________________________________________
*** CID 1362014:    (RESOURCE_LEAK)
/src/source-af-packet.c: 1799 in AFPSetupRing()
1793             }
1794             ptv->frame_offset = 0;
1795     #ifdef HAVE_TPACKET_V3
1796         }
1797     #endif
1798     
>>>     CID 1362014:    (RESOURCE_LEAK)
>>>     Variable "ring_buf" going out of scope leaks the storage it points to.
1799         return 0;
1800     
1801     mmap_err:
1802         /* Packet mmap does the cleaning when socket is closed */
1803         return AFP_FATAL_ERROR;
1804     }
/src/source-af-packet.c: 1803 in AFPSetupRing()
1797     #endif
1798     
1799         return 0;
1800     
1801     mmap_err:
1802         /* Packet mmap does the cleaning when socket is closed */
>>>     CID 1362014:    (RESOURCE_LEAK)
>>>     Variable "ring_buf" going out of scope leaks the storage it points to.
1803         return AFP_FATAL_ERROR;
1804     }
1805     
1806     static int AFPCreateSocket(AFPThreadVars *ptv, char *devname, int verbose)
1807     {
1808         int r;
Actions #1

Updated by Eric Leblond almost 8 years ago

  • % Done changed from 0 to 90
Actions #2

Updated by Victor Julien almost 8 years ago

  • Status changed from Assigned to Closed
  • Target version changed from 70 to 3.1rc1
  • % Done changed from 90 to 100
Actions #3

Updated by Victor Julien almost 8 years ago

  • Status changed from Closed to Assigned

Coverity still thinks we leak memory.

1637static int AFPSetupRing(AFPThreadVars *ptv, char *devname)
1638{
1639    int val;
1640    unsigned int len = sizeof(val), i;
1641    unsigned int ring_buflen;
1642    uint8_t * ring_buf;
1643    int order;
1644    int r, mmap_flag;
1645
1646#ifdef HAVE_TPACKET_V3
    1. Condition ptv->flags & (16 /* 1 << 4 */), taking false branch
1647    if (ptv->flags & AFP_TPACKET_V3) {
1648        val = TPACKET_V3;
1649    } else
1650#endif
1651    {
1652        val = TPACKET_V2;
1653    }
    2. Condition getsockopt(ptv->socket, 263, 11, &val, &len) < 0, taking false branch
1654    if (getsockopt(ptv->socket, SOL_PACKET, PACKET_HDRLEN, &val, &len) < 0) {
1655        if (errno == ENOPROTOOPT) {
1656            if (ptv->flags & AFP_TPACKET_V3) {
1657                SCLogError(SC_ERR_AFP_CREATE,
1658                        "Too old kernel giving up (need 3.2 for TPACKET_V3)");
1659            } else {
1660                SCLogError(SC_ERR_AFP_CREATE,
1661                        "Too old kernel giving up (need 2.6.27 at least)");
1662            }
1663        }
1664        SCLogError(SC_ERR_AFP_CREATE, "Error when retrieving packet header len");
1665        return AFP_FATAL_ERROR;
1666    }
1667
1668    val = TPACKET_V2;
1669#ifdef HAVE_TPACKET_V3
    3. Condition ptv->flags & (16 /* 1 << 4 */), taking false branch
1670    if (ptv->flags & AFP_TPACKET_V3) {
1671        val = TPACKET_V3;
1672    }
1673#endif
    4. Condition setsockopt(ptv->socket, 263, 10, &val, 4U /* sizeof (val) */) < 0, taking false branch
1674    if (setsockopt(ptv->socket, SOL_PACKET, PACKET_VERSION, &val,
1675                sizeof(val)) < 0) {
1676        SCLogError(SC_ERR_AFP_CREATE,
1677                "Can't activate TPACKET_V2/TPACKET_V3 on packet socket: %s",
1678                strerror(errno));
1679        return AFP_FATAL_ERROR;
1680    }
1681
1682#ifdef HAVE_HW_TIMESTAMPING
1683    int req = SOF_TIMESTAMPING_RAW_HARDWARE;
    5. Condition setsockopt(ptv->socket, 263, 17, (void *)&req, 4U /* sizeof (req) */) < 0, taking false branch
1684    if (setsockopt(ptv->socket, SOL_PACKET, PACKET_TIMESTAMP, (void *) &req,
1685                sizeof(req)) < 0) {
1686        SCLogWarning(SC_ERR_AFP_CREATE,
1687                "Can't activate hardware timestamping on packet socket: %s",
1688                strerror(errno));
1689    }
1690#endif
1691
1692    /* Allocate RX ring */
1693#ifdef HAVE_TPACKET_V3
    6. Condition ptv->flags & (16 /* 1 << 4 */), taking false branch
1694    if (ptv->flags & AFP_TPACKET_V3) {
1695        if (AFPComputeRingParamsV3(ptv) != 1) {
1696            return AFP_FATAL_ERROR;
1697        }
1698        r = setsockopt(ptv->socket, SOL_PACKET, PACKET_RX_RING,
1699                (void *) &ptv->req3, sizeof(ptv->req3));
1700        if (r < 0) {
1701            SCLogError(SC_ERR_MEM_ALLOC,
1702                    "Unable to allocate RX Ring for iface %s: (%d) %s",
1703                    devname,
1704                    errno,
1705                    strerror(errno));
1706            return AFP_FATAL_ERROR;
1707        }
1708    } else {
1709#endif
    7. Condition order >= 0, taking true branch
1710        for (order = AFP_BLOCK_SIZE_DEFAULT_ORDER; order >= 0; order--) {
    8. Condition AFPComputeRingParams(ptv, order) != 1, taking false branch
1711            if (AFPComputeRingParams(ptv, order) != 1) {
1712                SCLogInfo("Ring parameter are incorrect. Please correct the devel");
1713                return AFP_FATAL_ERROR;
1714            }
1715
1716            r = setsockopt(ptv->socket, SOL_PACKET, PACKET_RX_RING,
1717                    (void *) &ptv->req, sizeof(ptv->req));
1718
    9. Condition r < 0, taking false branch
1719            if (r < 0) {
1720                if (errno == ENOMEM) {
1721                    SCLogInfo("Memory issue with ring parameters. Retrying.");
1722                    continue;
1723                }
1724                SCLogError(SC_ERR_MEM_ALLOC,
1725                        "Unable to allocate RX Ring for iface %s: (%d) %s",
1726                        devname,
1727                        errno,
1728                        strerror(errno));
1729                return AFP_FATAL_ERROR;
1730            } else {
    10. Breaking from loop
1731                break;
1732            }
1733        }
    11. Condition order < 0, taking false branch
1734        if (order < 0) {
1735            SCLogError(SC_ERR_MEM_ALLOC,
1736                    "Unable to allocate RX Ring for iface %s (order 0 failed)",
1737                    devname);
1738            return AFP_FATAL_ERROR;
1739        }
1740#ifdef HAVE_TPACKET_V3
1741    }
1742#endif
1743
1744    /* Allocate the Ring */
1745#ifdef HAVE_TPACKET_V3
    12. Condition ptv->flags & (16 /* 1 << 4 */), taking false branch
1746    if (ptv->flags & AFP_TPACKET_V3) {
1747        ring_buflen = ptv->req3.tp_block_nr * ptv->req3.tp_block_size;
1748    } else {
1749#endif
1750        ring_buflen = ptv->req.tp_block_nr * ptv->req.tp_block_size;
1751#ifdef HAVE_TPACKET_V3
1752    }
1753#endif
1754    mmap_flag = MAP_SHARED;
    13. Condition ptv->flags & (64 /* 1 << 6 */), taking false branch
1755    if (ptv->flags & AFP_MMAP_LOCKED)
1756        mmap_flag |= MAP_LOCKED;
    14. alloc_fn: Storage is returned from allocation function mmap.
    15. var_assign: Assigning: ring_buf = storage returned from mmap(NULL, ring_buflen, 3, mmap_flag, ptv->socket, 0L).
1757    ring_buf = mmap(0, ring_buflen, PROT_READ|PROT_WRITE,
1758            mmap_flag, ptv->socket, 0);
    16. Condition ring_buf == (void *)0xffffffffffffffff, taking false branch
1759    if (ring_buf == MAP_FAILED) {
1760        SCLogError(SC_ERR_MEM_ALLOC, "Unable to mmap, error %s",
1761                   strerror(errno));
1762        goto mmap_err;
1763    }
1764#ifdef HAVE_TPACKET_V3
    17. Condition ptv->flags & (16 /* 1 << 4 */), taking false branch
1765    if (ptv->flags & AFP_TPACKET_V3) {
1766        ptv->ring_v3 = SCMalloc(ptv->req3.tp_block_nr * sizeof(*ptv->ring_v3));
1767        if (!ptv->ring_v3) {
1768            SCLogError(SC_ERR_MEM_ALLOC, "Unable to malloc ptv ring_v3");
1769            goto postmmap_err;
1770        }
1771        for (i = 0; i < ptv->req3.tp_block_nr; ++i) {
1772            ptv->ring_v3[i].iov_base = ring_buf + (i * ptv->req3.tp_block_size);
1773            ptv->ring_v3[i].iov_len = ptv->req3.tp_block_size;
1774        }
1775    } else {
1776#endif
1777        /* allocate a ring for each frame header pointer*/
    18. Condition ptrmem == NULL, taking false branch
1778        ptv->ring_v2 = SCMalloc(ptv->req.tp_frame_nr * sizeof (union thdr *));
    19. Condition (*ptv).ring_v2 == NULL, taking false branch
1779        if (ptv->ring_v2 == NULL) {
1780            SCLogError(SC_ERR_MEM_ALLOC, "Unable to allocate frame buf");
1781            goto postmmap_err;
1782        }
1783        memset(ptv->ring_v2, 0, ptv->req.tp_frame_nr * sizeof (union thdr *));
1784        /* fill the header ring with proper frame ptr*/
1785        ptv->frame_offset = 0;
    20. Condition i < (*ptv).req.tp_block_nr, taking true branch
    25. Condition i < (*ptv).req.tp_block_nr, taking false branch
1786        for (i = 0; i < ptv->req.tp_block_nr; ++i) {
    21. var_assign: Assigning: base = ring_buf.
1787            void *base = &ring_buf[i * ptv->req.tp_block_size];
1788            unsigned int j;
    22. Condition j < (*ptv).req.tp_block_size / (*ptv).req.tp_frame_size, taking false branch
1789            for (j = 0; j < ptv->req.tp_block_size / ptv->req.tp_frame_size; ++j, ++ptv->frame_offset) {
1790                (((union thdr **)ptv->ring_v2)[ptv->frame_offset]) = base;
1791                base += ptv->req.tp_frame_size;
1792            }
    23. leaked_storage: Variable base going out of scope leaks the storage it points to.
    24. Jumping back to the beginning of the loop
1793        }
1794        ptv->frame_offset = 0;
1795#ifdef HAVE_TPACKET_V3
1796    }
1797#endif
1798
    CID 1362014 (#1 of 1): Resource leak (RESOURCE_LEAK)26. leaked_storage: Variable ring_buf going out of scope leaks the storage it points to.
1799    return 0;
1800
1801postmmap_err:
1802    munmap(ring_buf, ring_buflen);
1803    if (ptv->ring_v2)
1804        SCFree(ptv->ring_v2);
1805    if (ptv->ring_v3)
1806        SCFree(ptv->ring_v3);
1807mmap_err:
1808    /* Packet mmap does the cleaning when socket is closed */
1809    return AFP_FATAL_ERROR;
1810}
Actions #4

Updated by Victor Julien almost 8 years ago

  • Target version changed from 3.1rc1 to 70
Actions #5

Updated by Victor Julien about 7 years ago

Possibly related:

Indirect leak of 560000 byte(s) in 8 object(s) allocated from:
    #0 0x7f6d14c3f602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0x7e719c in ReceiveAFPThreadInit /home/victor/dev/suricata/src/source-af-packet.c:2131
    #2 0x870e64 in TmThreadsSlotPktAcqLoop /home/victor/dev/suricata/src/tm-threads.c:294
    #3 0x7f6d132796b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

Indirect leak of 16640 byte(s) in 8 object(s) allocated from:
    #0 0x7f6d14c3f602 in malloc (/usr/lib/x86_64-linux-gnu/libasan.so.2+0x98602)
    #1 0x7e291c in AFPSetupRing /home/victor/dev/suricata/src/source-af-packet.c:1775
    #2 0x7e509a in AFPCreateSocket /home/victor/dev/suricata/src/source-af-packet.c:1954
    #3 0x7dd458 in ReceiveAFPLoop /home/victor/dev/suricata/src/source-af-packet.c:1333
    #4 0x8712fe in TmThreadsSlotPktAcqLoop /home/victor/dev/suricata/src/tm-threads.c:334
    #5 0x7f6d132796b9 in start_thread (/lib/x86_64-linux-gnu/libpthread.so.0+0x76b9)

Actions #6

Updated by Victor Julien almost 6 years ago

  • Status changed from Assigned to Closed
  • Target version changed from 70 to 4.1beta1

This issue has been fixed.

Actions

Also available in: Atom PDF