Actions
Bug #6825
closedaf-packet: possible free of unallocated memory
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