Project

General

Profile

Actions

Bug #1062

closed

bad realloc usage pattern

Added by Victor Julien over 10 years ago. Updated almost 8 years ago.

Status:
Closed
Priority:
Normal
Assignee:
-
Target version:
-
Affected Versions:
Effort:
Difficulty:
Label:

Description

In quite a few places, we have the following pattern:

ptr = realloc(ptr, newsize);

However, if realloc fails we're in trouble: "If realloc() fails the original block is left untouched; it is not freed or moved." (man realloc)

As we then overwrite the pointer to NULL, we've lost our reference to the original allocation, leading to memory leaking.

Actions #1

Updated by Eric Leblond over 10 years ago

  • Status changed from New to Assigned
  • Assignee changed from OISF Dev to Eric Leblond

Fix to be made via a cocci patch. And QA script to be provided.

Actions #2

Updated by Victor Julien over 10 years ago

  • Target version changed from 2.0rc1 to 2.0beta2
Actions #3

Updated by Eric Leblond over 10 years ago

  • Status changed from Assigned to Closed
Actions #4

Updated by Victor Julien over 10 years ago

  • Status changed from Closed to Assigned
  • Target version changed from 2.0beta2 to 2.0rc1
  • % Done changed from 0 to 80

The fix is incomplete. It seems to have missed those cases where the realloc is done inside a macro in a .h file. E.g.:

app-layer-smtp.c:589:13: warning: Potential leak of memory pointed to by field 'events'
            AppLayerDecoderEventsSetEvent(f,
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
./decode-events.h:322:47: note: expanded from macro 'AppLayerDecoderEventsSetEvent'
                devents->events_buffer_size = 0;                        \
                                              ^
1 warning generated.

Actions #5

Updated by Victor Julien about 10 years ago

  • Target version changed from 2.0rc1 to 3.0RC2

IIRC a coccinelle issue prevents us from doing proper QA on the macro's. Will revisit later.

Actions #6

Updated by Victor Julien over 8 years ago

  • Target version changed from 3.0RC2 to 70
Actions #7

Updated by Victor Julien almost 8 years ago

  • Status changed from Assigned to Closed
  • Assignee deleted (Eric Leblond)
  • Target version deleted (70)

Think it's safe to close this now.

Actions

Also available in: Atom PDF