Suricata

Bug #6825

Updated by Victor Julien about 1 month ago

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. 

 <pre><code class="c"> <pre> 
 #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; 
 } 
 </code></pre> </pre> 

 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.

Back