Skip to content

Commit

Permalink
app-layer: Set sc_errno upon error return
Browse files Browse the repository at this point in the history
Bug: https://redmine.openinfosecfoundation.org/issues/6782

Callers to these allocators often use ``sc_errno`` to provide context of
the error. And in the case of the above bug, they return ``sc_errno``,
but as it has not been set ``sc_errno = 0; == SC_OK``.

This patch simply sets this variable to ensure there is context provided
upon error.
  • Loading branch information
rmcconnell-r7 authored and victorjulien committed May 22, 2024
1 parent 52a008e commit fc2e49f
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 10 deletions.
16 changes: 12 additions & 4 deletions src/app-layer-ftp.c
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,17 @@ static int FTPCheckMemcap(uint64_t size)

static void *FTPCalloc(size_t n, size_t size)
{
if (FTPCheckMemcap((uint32_t)(n * size)) == 0)
if (FTPCheckMemcap((uint32_t)(n * size)) == 0) {
sc_errno = SC_ELIMIT;
return NULL;
}

void *ptr = SCCalloc(n, size);

if (unlikely(ptr == NULL))
if (unlikely(ptr == NULL)) {
sc_errno = SC_ENOMEM;
return NULL;
}

FTPIncrMemuse((uint64_t)(n * size));
return ptr;
Expand All @@ -208,12 +212,16 @@ static void *FTPRealloc(void *ptr, size_t orig_size, size_t size)
{
void *rptr = NULL;

if (FTPCheckMemcap((uint32_t)(size - orig_size)) == 0)
if (FTPCheckMemcap((uint32_t)(size - orig_size)) == 0) {
sc_errno = SC_ELIMIT;
return NULL;
}

rptr = SCRealloc(ptr, size);
if (rptr == NULL)
if (rptr == NULL) {
sc_errno = SC_ENOMEM;
return NULL;
}

if (size > orig_size) {
FTPIncrMemuse(size - orig_size);
Expand Down
24 changes: 18 additions & 6 deletions src/app-layer-htp-mem.c
Original file line number Diff line number Diff line change
Expand Up @@ -136,13 +136,17 @@ void *HTPMalloc(size_t size)
{
void *ptr = NULL;

if (HTPCheckMemcap((uint32_t)size) == 0)
if (HTPCheckMemcap((uint32_t)size) == 0) {
sc_errno = SC_ELIMIT;
return NULL;
}

ptr = SCMalloc(size);

if (unlikely(ptr == NULL))
if (unlikely(ptr == NULL)) {
sc_errno = SC_ENOMEM;
return NULL;
}

HTPIncrMemuse((uint64_t)size);

Expand All @@ -153,13 +157,17 @@ void *HTPCalloc(size_t n, size_t size)
{
void *ptr = NULL;

if (HTPCheckMemcap((uint32_t)(n * size)) == 0)
if (HTPCheckMemcap((uint32_t)(n * size)) == 0) {
sc_errno = SC_ELIMIT;
return NULL;
}

ptr = SCCalloc(n, size);

if (unlikely(ptr == NULL))
if (unlikely(ptr == NULL)) {
sc_errno = SC_ENOMEM;
return NULL;
}

HTPIncrMemuse((uint64_t)(n * size));

Expand All @@ -169,13 +177,17 @@ void *HTPCalloc(size_t n, size_t size)
void *HTPRealloc(void *ptr, size_t orig_size, size_t size)
{
if (size > orig_size) {
if (HTPCheckMemcap((uint32_t)(size - orig_size)) == 0)
if (HTPCheckMemcap((uint32_t)(size - orig_size)) == 0) {
sc_errno = SC_ELIMIT;
return NULL;
}
}

void *rptr = SCRealloc(ptr, size);
if (rptr == NULL)
if (rptr == NULL) {
sc_errno = SC_ENOMEM;
return NULL;
}

if (size > orig_size) {
HTPIncrMemuse((uint64_t)(size - orig_size));
Expand Down
48 changes: 48 additions & 0 deletions src/util-streaming-buffer.c
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@
#include "util-debug.h"
#include "util-error.h"

#include "app-layer-htp-mem.h"
#include "conf-yaml-loader.h"
#include "app-layer-htp.h"

static void ListRegions(StreamingBuffer *sb);

#define DUMP_REGIONS 0 // set to 1 to dump a visual representation of the regions list and sbb tree.
Expand Down Expand Up @@ -721,6 +725,8 @@ static inline int WARN_UNUSED GrowRegionToSize(StreamingBuffer *sb,

void *ptr = REALLOC(cfg, region->buf, region->buf_size, grow);
if (ptr == NULL) {
if (sc_errno == SC_OK)
sc_errno = SC_ENOMEM;
return sc_errno;
}
/* for safe printing and general caution, lets memset the
Expand Down Expand Up @@ -1099,6 +1105,8 @@ int StreamingBufferAppend(StreamingBuffer *sb, const StreamingBufferConfig *cfg,
}
}
DEBUG_VALIDATE_BUG_ON(DataFits(sb, data_len) != 1);
if (DataFits(sb, data_len) != 1)
return -1;

memcpy(sb->region.buf + sb->region.buf_offset, data, data_len);
seg->stream_offset = sb->region.stream_offset + sb->region.buf_offset;
Expand Down Expand Up @@ -2366,6 +2374,45 @@ static int StreamingBufferTest11(void)
StreamingBufferFree(sb, &cfg);
PASS;
}

static const char *dummy_conf_string = "%YAML 1.1\n"
"---\n"
"\n"
"app-layer:\n"
" protocols:\n"
" http:\n"
" enabled: yes\n"
" memcap: 88\n"
"\n";

static int StreamingBufferTest12(void)
{
ConfCreateContextBackup();
ConfInit();
HtpConfigCreateBackup();
ConfYamlLoadString((const char *)dummy_conf_string, strlen(dummy_conf_string));
HTPConfigure();

StreamingBufferConfig cfg = { 8, 1, STREAMING_BUFFER_REGION_GAP_DEFAULT, HTPCalloc, HTPRealloc,
HTPFree };
StreamingBuffer *sb = StreamingBufferInit(&cfg);
FAIL_IF(sb == NULL);

StreamingBufferSegment seg1;
FAIL_IF(StreamingBufferAppend(sb, &cfg, &seg1, (const uint8_t *)"ABCDEFGHIJKLMNOP", 16) != 0);

StreamingBufferSegment seg2;
FAIL_IF(StreamingBufferAppend(sb, &cfg, &seg2,
(const uint8_t *)"ABCDEFGHIJKLMNOPQRSTUVWXYZABCDEFGHIJKLMNOPQRSTUVWXYZ",
52) != -1);
FAIL_IF(sc_errno != SC_ELIMIT);

StreamingBufferFree(sb, &cfg);
HtpConfigRestoreBackup();
ConfRestoreContextBackup();

PASS;
}
#endif

void StreamingBufferRegisterTests(void)
Expand All @@ -2380,5 +2427,6 @@ void StreamingBufferRegisterTests(void)
UtRegisterTest("StreamingBufferTest09", StreamingBufferTest09);
UtRegisterTest("StreamingBufferTest10", StreamingBufferTest10);
UtRegisterTest("StreamingBufferTest11 Bug 6903", StreamingBufferTest11);
UtRegisterTest("StreamingBufferTest12 Bug 6782", StreamingBufferTest12);
#endif
}

0 comments on commit fc2e49f

Please sign in to comment.