Project

General

Profile

Actions

Bug #6825

closed

af-packet: possible free of unallocated memory

Added by Alexey Simakov 8 months ago. Updated 5 months ago.

Status:
Rejected
Priority:
Normal
Target version:
Affected Versions:
Effort:
low
Difficulty:
low
Label:

Description

AFPSetupRing function allocating memory for ptv->ring.v2/3 pointer.
In case of failing of allocating memory jump to postmap_err point happening,
where its try to free ptv->ring.v2/v3 field with check on nulls.
For ptr, which have should store allocated memory nothing happened, since
unsuccessful malloc call usually return NULL, but there is also try to
release memory for another field, which is uninitialized and in general
can contain any part of memory.

#ifdef HAVE_TPACKET_V3
    if (ptv->flags & AFP_TPACKET_V3) {
        ptv->ring.v3 = SCMalloc(ptv->req.v3.tp_block_nr * sizeof(*ptv->ring.v3)); <--- Trying to allocate memory for ptv->ring.v3
        if (!ptv->ring.v3) {
            SCLogError("%s: failed to alloc ring: %s", devname, strerror(errno));
            goto postmmap_err; <--- Allocating failed. Jump to postmmap_err label
        }
        for (i = 0; i < ptv->req.v3.tp_block_nr; ++i) {
            ptv->ring.v3[i].iov_base = ptv->ring_buf + (i * ptv->req.v3.tp_block_size);
            ptv->ring.v3[i].iov_len = ptv->req.v3.tp_block_size;
        }
    } else {
#endif
        /* allocate a ring for each frame header pointer*/
        ptv->ring.v2 = SCCalloc(ptv->req.v2.tp_frame_nr, sizeof(union thdr *));
        if (ptv->ring.v2 == NULL) {
            SCLogError("%s: failed to alloc ring: %s", devname, strerror(errno));
            goto postmmap_err;
        }
        /* fill the header ring with proper frame ptr*/
        ptv->frame_offset = 0;
        for (i = 0; i < ptv->req.v2.tp_block_nr; ++i) {
            void *base = &(ptv->ring_buf[i * ptv->req.v2.tp_block_size]);
            unsigned int j;
            for (j = 0; j < ptv->req.v2.tp_block_size / ptv->req.v2.tp_frame_size; ++j, ++ptv->frame_offset) {
                (((union thdr **)ptv->ring.v2)[ptv->frame_offset]) = base;
                base += ptv->req.v2.tp_frame_size;
            }
        }
        ptv->frame_offset = 0;
#ifdef HAVE_TPACKET_V3
    }
#endif

    return 0;

postmmap_err:
    munmap(ptv->ring_buf, ptv->ring_buflen);
    if (ptv->ring.v2) <--- Dangerous ptv->ring.v2 is uninitialized
        SCFree(ptv->ring.v2);
    if (ptv->ring.v3) <--- Its ok, since ptv->ring.v3 in NULL state after unsuccessful allocating
        SCFree(ptv->ring.v3);
mmap_err:
    /* Packet mmap does the cleaning when socket is closed */
    return AFP_FATAL_ERROR;
}

Probably, this is safe for most modern compilers since AFAIK they initialize all pointer to NULL by default,
but its not possible to rely on this behavior since its not standardized.

Actions

Also available in: Atom PDF