af-packet: fix looping in ring buffer.

A crash can occurs in the following conditions:
 * Suricata running in other mode than "workers"
 * Kernel fill in the ring buffer
Under this conditions, it is possible that the capture thread reads
a packet that has not yet released by one of the treatment threads
because there is no modification done on the ring buffer entry when
a packet is read. Doing, this it access to memory which can be
released to the kernel and modified. This results in a kind of memory
corruption.

This bug has only been seen recently and this has to be linked with the
read speed improvement recently made in AF_PACKET support.

The patch fixes the issue by modifying the tp_status bitmask in the
ring buffer. It sets the TP_STATUS_USER_BUSY flag when it is confirmed
that the packet will be treated. And at the start of the read, it exits
from the reading loop (returning to poll) when it reaches a packet with
the flag set. As tp_status is set to 0 during packet release the flag
is destroyed when releasing the packet.

Regarding concurrency, we've got a sequence of modification. The
capture thread read the packet and set the flag, then it passes the
queue and the packet get processed by other threads. The change on
tp_status are thus made at different time.

Regarding the value of the flag, the patch uses the last bit of
tp_status to avoid be impacting by a change in kernel. I will
propose a patch to have TP_STATUS_USER_BUSY included in kernel
as this is a generic issue for multithreading application using
AF_PACKET mechanism.

diff --git a/src/source-af-packet.c b/src/source-af-packet.c
index bdad54048bd582ff88ed2cf774684fe12d505bbd..904d571f5d93628efa28349bcb3095f8bd1568e2 100644 (file)
--- a/src/source-af-packet.c
+++ b/src/source-af-packet.c
@@ -124,6 +124,11 @@ TmEcode NoAFPSupportExit(ThreadVars *tv, void *initdata, void **data)
 
 #define POLL_TIMEOUT 100
 
+#ifndef TP_STATUS_USER_BUSY
+/* for new use latest bit available in tp_status */
+#define TP_STATUS_USER_BUSY (1 << 31)
+#endif
+
 /** protect pfring_set_bpf_filter, as it is not thread safe */
 static SCMutex afpacket_bpf_set_filter_lock = PTHREAD_MUTEX_INITIALIZER;
 
@@ -696,6 +701,12 @@ int AFPReadFromRing(AFPThreadVars *ptv)
 
         read_pkts++;
 
+        /* Our packet is still used by suricata, we exit read loop to
+         * gain some time */
+        if (h.h2->tp_status & TP_STATUS_USER_BUSY) {
+            SCReturnInt(AFP_READ_OK);
+        }
+
         if ((ptv->flags & AFP_EMERGENCY_MODE) && (emergency_flush == 1)) {
             h.h2->tp_status = TP_STATUS_KERNEL;
             goto next_frame;
@@ -706,6 +717,11 @@ int AFPReadFromRing(AFPThreadVars *ptv)
             SCReturnInt(AFP_FAILURE);
         }
 
+        /* Suricata will treat packet so telling it is busy, this
+         * status will be reset to 0 (ie TP_STATUS_KERNEL) in the release
+         * function. */
+        h.h2->tp_status |= TP_STATUS_USER_BUSY;
+
         p->afp_v.relptr = h.raw;
         p->ReleaseData = AFPReleaseDataFromRing;
         p->afp_v.mpeer = ptv->mpeer;