Project

General

Profile

Actions
Copy link

Bug #2584

closed

Patch for Resource leak

Added by shashank1.m over 10 years ago. Updated over 8 years ago.

Status:
Fixed
Priority:
Normal
Category:
core
Target version:
1.4.40
ASK QUESTIONS IN Forums:

Description

  • lighttpd-1.4.35/src/lemon.c
    Bug Type: Resource leak
    Description: memory is assigned to path in pathsearch(), and then path is returned to tpltname. If fopen() fails to
    open template file, then it should be freed before going this memory out of scope. Currently It is not
    handled in case of failure.
  • lighttpd-1.4.35/src/configfile.c
    Bug Type: Dereference null return value
    Description: configparserAlloc() will return null if malloc() fails, so there should be check if pParser is null or
    not before using it.
  • lighttpd-1.4.35/src/http_auth.c
    Bug Type: Dereference null return value
    Description: array_get_element() may return null, so there should be null check before using it.
  • lighttpd-1.4.35/src/keyvalue.c
    Bug Type: Dereference null return value
    Description: realloc() may fails to allocate memory, will return null. So there should be check before using it.
  • lighttpd-1.4.35/src/mod_accesslog.c
    Bug Type: Dereference null return value
    Description: realloc() may fails to allocate memory, will return null. So there should be check before using it.
  • lighttpd-1.4.35/src/mod_auth.c
    Bug Type: Dereference null return value
    Description: array_get_element() may return null, so there should be null check before using it.
  • lighttpd-1.4.35/src/mod_dirlisting.c
    Bug Type: Dereference null return value
    Description: realloc() may fails to allocate memory, will return null. So there should be check before using it.
  • lighttpd-1.4.35/src/mod_rewrite.c
    Bug Type: Dereference null return value
    Description: realloc() may fails to allocate memory, will return null. So there should be check before using it.
  • lighttpd-1.4.35/src/mod_ssi_expr.c
    Bug Type: Dereference null return value
    Description: ssiexprparserAlloc() will return null if malloc() fails, so there should be check if pParser is null
    or not before using it.

Files

Download all files
lightty_patch.patch (5.14 KB) lightty_patch.patch This patch will contains the fixes of above memory leak, Dereference null return value errors. shashank1.m, 2014-07-07 14:38
lightty_patch.patch (5.14 KB) lightty_patch.patch Patch submitted for review shashank1.m, 2014-07-07 14:43
lighttpd_new_patch.patch (2.89 KB) lighttpd_new_patch.patch shashank1.m, 2014-07-08 06:27

Related issues 1 (0 open — 1 closed)

Has duplicate Bug #2581: Dereference a null pointerDuplicate2014-07-072014-07-14Actions
Actions
Copy link
 #1

Updated by shashank1.m over 10 years ago

Actions
Copy link
 #2

Updated by stbuehler over 10 years ago

Hi, thx for bringing these problems to our attention. I'll answer here too for #2581, #2582 and #2583, because I can't see a logical reason to split them.

  • lemon.c: this is just a parser generator; we don't care much about resource leaks or how to handle alloc failures (abort() or segfault.. whatever)
  • get_http_version_name and get_http_method_name: if these return NULL there is something seriously wrong (most likely a memory corruption) - "ignoring" this is imho a very bad idea; i'd rather segfault (or `force_assert`).
  • http_auth.c: mod_auth checks those two lookups are good at "configure" stage; if these break later there must be some serious problem; again I prefer a segfault or assert
  • ignoring a `realloc` failure leaks memory!
  • allocations should probably be checked with `force_assert`; if you can't allocate memory anymore it is very likely you get killed by the kernel soon anyway

I can't see anything serious in these reports; I probably should take a look at them again after I completed the work on my branch http://git.lighttpd.net/lighttpd/lighttpd-1.x.git/log/?h=lighttpd-1.4.x-stbuehler-api-cleanup

Actions
Copy link
 #3

Updated by shashank1.m over 10 years ago

Thanks for your input.
As per your suggestions I am attaching new patch. Please review.

Actions
Copy link
 #4

Updated by stbuehler over 9 years ago

  • Target version changed from 1.4.36 to 1.4.37
Actions
Copy link
 #5

Updated by stbuehler about 9 years ago

  • Target version changed from 1.4.37 to 1.4.38
Actions
Copy link
 #6

Updated by stbuehler about 9 years ago

  • Target version changed from 1.4.38 to 1.4.39
Actions
Copy link
 #7

Updated by stbuehler almost 9 years ago

  • Target version changed from 1.4.39 to 1.4.40
Actions
Copy link
 #8

Updated by gstrauss over 8 years ago

See recent comments in https://redmine.lighttpd.net/issues/2583

In the past few months, there have been numerous commits sprinkling more force_assert()s in the code.

Is this ticket a generic placeholder, or can this ticket be closed?

Actions
Copy link
 #9

Updated by stbuehler over 8 years ago

  • Has duplicate Bug #2581: Dereference a null pointer added
Actions
Copy link
 #10

Updated by gstrauss over 8 years ago

x-ref: https://redmine.lighttpd.net/boards/3/topics/6333 offers a large patch which adds lots of force_assert() for many/all memory allocations. That patch might be useful for debugging memory issues, but probably shouldn't be applied using force_assert(), which is always active at runtime. Maybe a debug_mem_assert() which could be enabled with a make macro/define?

Actions
Copy link
 #11

Updated by gstrauss over 8 years ago

  • Status changed from Patch Pending to Fixed

The specific items identified in lighttpd_new_patch.patch have been addressed.

Actions
Copy link

Also available in: Atom