Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Account for allocated heap memory which is unreachable from globals #1241

Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 115 additions & 6 deletions src/analyses/memLeak.ml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,105 @@
let context _ d = d

(* HELPER FUNCTIONS *)
let get_global_vars () =
(* Filtering by GVar seems to account for declarations, as well as definitions of global vars *)
List.filter_map (function GVar (v, _, _) -> Some v | _ -> None) !Cilfacade.current_file.globals
mrstanb marked this conversation as resolved.
Show resolved Hide resolved

let get_global_struct_ptr_vars () =
List.filter_map (function GVar (v, _, _) -> Some v | _ -> None) !Cilfacade.current_file.globals
|> List.filter (fun v -> match v.vtype with TPtr (TComp _, _) | TPtr ((TNamed ({ttype = TComp _; _}, _)), _) -> true | _ -> false)
mrstanb marked this conversation as resolved.
Show resolved Hide resolved

let get_global_struct_non_ptr_vars () =
List.filter_map (function GVar (v, _, _) -> Some v | _ -> None) !Cilfacade.current_file.globals
|> List.filter (fun v -> match v.vtype with TComp (_, _) | (TNamed ({ttype = TComp _; _}, _)) -> true | _ -> false)
michael-schwarz marked this conversation as resolved.
Show resolved Hide resolved
mrstanb marked this conversation as resolved.
Show resolved Hide resolved

let get_reachable_mem_from_globals (global_vars:varinfo list) ctx =
global_vars
|> List.map (fun v -> Lval (Var v, NoOffset))
|> List.filter_map (fun exp ->
michael-schwarz marked this conversation as resolved.
Show resolved Hide resolved
match ctx.ask (Queries.MayPointTo exp) with
| a when not (Queries.AD.is_top a) && Queries.AD.cardinal a = 1 ->
begin match List.hd @@ Queries.AD.elements a with
| Queries.AD.Addr.Addr (v, _) when (ctx.ask (Queries.IsHeapVar v)) && not (ctx.ask (Queries.IsMultiple v)) -> Some v
| _ -> None
end
| _ -> None)

let rec get_reachable_mem_from_str_ptr_globals (global_struct_ptr_vars:varinfo list) ctx =
let eval_value_of_heap_var heap_var =
match ctx.ask (Queries.EvalValue (Lval (Var heap_var, NoOffset))) with
| a when not (Queries.VD.is_top a) ->
begin match a with
| Struct s ->
List.fold_left (fun acc f ->
if isPointerType f.ftype then
begin match ValueDomain.Structs.get s f with
| Queries.VD.Address a ->
let reachable_from_addr_set =
List.fold_left (fun acc addr ->
mrstanb marked this conversation as resolved.
Show resolved Hide resolved
match addr with
| Queries.AD.Addr.Addr (v, _) -> List.append acc (v :: get_reachable_mem_from_str_ptr_globals [v] ctx)
mrstanb marked this conversation as resolved.
Show resolved Hide resolved
| _ -> acc
) [] (Queries.AD.elements a)
mrstanb marked this conversation as resolved.
Show resolved Hide resolved
in List.append acc reachable_from_addr_set
| _ -> acc
end
else acc
) [] (ValueDomain.Structs.keys s)
| _ -> []
end
| _ -> []
in
let get_pts_of_non_heap_ptr_var var =
match ctx.ask (Queries.MayPointTo (Lval (Var var, NoOffset))) with
| a when not (Queries.AD.is_top a) && Queries.AD.cardinal a = 1 ->
begin match List.hd @@ Queries.AD.elements a with
| Queries.AD.Addr.Addr (v, _) when (ctx.ask (Queries.IsHeapVar v)) && not (ctx.ask (Queries.IsMultiple v)) -> v :: (eval_value_of_heap_var v)
| Queries.AD.Addr.Addr (v, _) when not (ctx.ask (Queries.IsAllocVar v)) && isPointerType v.vtype -> get_reachable_mem_from_str_ptr_globals [v] ctx
| _ -> []
end
| _ -> []
in
global_struct_ptr_vars
|> List.fold_left (fun acc var ->
if ctx.ask (Queries.IsHeapVar var) then eval_value_of_heap_var var
else if not (ctx.ask (Queries.IsAllocVar var)) && isPointerType var.vtype then get_pts_of_non_heap_ptr_var var
else acc
mrstanb marked this conversation as resolved.
Show resolved Hide resolved
) []

let get_reachable_mem_from_str_non_ptr_globals (global_struct_non_ptr_vars:varinfo list) ctx =
global_struct_non_ptr_vars
(* Filter out global struct vars that don't have pointer fields *)
|> List.filter_map (fun v ->
match ctx.ask (Queries.EvalValue (Lval (Var v, NoOffset))) with
| a when not (Queries.VD.is_top a) ->
begin match a with
| Queries.VD.Struct s ->
let struct_fields = ValueDomain.Structs.keys s in
let ptr_struct_fields = List.filter (fun f -> isPointerType f.ftype) struct_fields in
if List.length ptr_struct_fields = 0 then None else Some (s, ptr_struct_fields)
github-advanced-security[bot] marked this conversation as resolved.
Fixed
Show resolved Hide resolved
| _ -> None
end
| _ -> None
)
|> List.fold_left (fun acc_struct (s, fields) ->
let reachable_from_fields =
List.fold_left (fun acc_field field ->
match ValueDomain.Structs.get s field with
| Queries.VD.Address a ->
let reachable_from_addr_set =
List.fold_left (fun acc_addr addr ->
match addr with
| Queries.AD.Addr.Addr (v, _) -> List.append acc_addr (v :: get_reachable_mem_from_str_ptr_globals [v] ctx)
| _ -> acc_addr
) [] (Queries.AD.elements a)
in List.append acc_field reachable_from_addr_set
| _ -> acc_field
) [] fields
in
List.append acc_struct reachable_from_fields
) []

let warn_for_multi_threaded ctx =
if not (ctx.ask (Queries.MustBeSingleThreaded { since_start = true })) then (
set_mem_safety_flag InvalidMemTrack;
Expand All @@ -28,17 +127,27 @@
)

let check_for_mem_leak ?(assert_exp_imprecise = false) ?(exp = None) ctx =
let state = ctx.local in
if not @@ D.is_empty state then
let allocated_mem = ctx.local in
if not (D.is_empty allocated_mem) then
let reachable_mem_from_non_struct_globals = D.of_list (get_reachable_mem_from_globals (get_global_vars ()) ctx) in
let reachable_mem_from_struct_ptr_globals = D.of_list (get_reachable_mem_from_str_ptr_globals (get_global_struct_ptr_vars ()) ctx) in
let reachable_mem_from_struct_non_ptr_globals = D.of_list (get_reachable_mem_from_str_non_ptr_globals (get_global_struct_non_ptr_vars ()) ctx) in
let reachable_mem_from_struct_globals = D.join reachable_mem_from_struct_ptr_globals reachable_mem_from_struct_non_ptr_globals in
let reachable_mem = D.join reachable_mem_from_non_struct_globals reachable_mem_from_struct_globals in
(* Check and warn if there's unreachable allocated memory at program exit *)
let allocated_and_unreachable_mem = D.diff allocated_mem reachable_mem in
if not (D.is_empty allocated_and_unreachable_mem) then (
set_mem_safety_flag InvalidMemTrack;
M.warn ~category:(Behavior (Undefined MemoryLeak)) ~tags:[CWE 401] "There is unreachable allocated heap memory at program exit. A memory leak might occur for the alloc vars %a\n" (Pretty.d_list ", " CilType.Varinfo.pretty) (D.elements allocated_and_unreachable_mem)
);
(* Check and warn if some of the allocated memory is not deallocated at program exit *)
match assert_exp_imprecise, exp with
| true, Some exp ->
set_mem_safety_flag InvalidMemTrack;
set_mem_safety_flag InvalidMemcleanup;
M.warn ~category:(Behavior (Undefined MemoryLeak)) ~tags:[CWE 401] "assert expression %a is unknown. Memory leak might possibly occur for heap variables: %a" d_exp exp D.pretty state
M.warn ~category:(Behavior (Undefined MemoryLeak)) ~tags:[CWE 401] "Assert expression %a is unknown. Memory leak might possibly occur for heap variables: %a" d_exp exp D.pretty allocated_mem
| _ ->
set_mem_safety_flag InvalidMemTrack;
set_mem_safety_flag InvalidMemcleanup;
M.warn ~category:(Behavior (Undefined MemoryLeak)) ~tags:[CWE 401] "Memory leak detected for heap variables: %a" D.pretty state
M.warn ~category:(Behavior (Undefined MemoryLeak)) ~tags:[CWE 401] "Memory leak detected for heap variables: %a" D.pretty allocated_mem

(* TRANSFER FUNCTIONS *)
let return ctx (exp:exp option) (f:fundec) : D.t =
Expand Down
12 changes: 12 additions & 0 deletions tests/regression/76-memleak/08-unreachable-mem.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
//PARAM: --set ana.malloc.unique_address_count 1 --set ana.activated[+] memLeak
#include <stdlib.h>

int *g;

int main(int argc, char const *argv[]) {
g = malloc(sizeof(int));
// Reference to g's heap contents is lost here
g = NULL;

return 0; //WARN
}
15 changes: 15 additions & 0 deletions tests/regression/76-memleak/09-unreachable-with-local-var.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
//PARAM: --set ana.malloc.unique_address_count 1 --set ana.activated[+] memLeak
#include <stdlib.h>

int *g;

int main(int argc, char const *argv[]) {
g = malloc(sizeof(int));
// Reference to g's heap contents is lost here
g = NULL;

// According to `valid-memtrack`, the memory of p is unreachable and we don't have a false positive
int *p = malloc(sizeof(int));

return 0; //WARN
}
16 changes: 16 additions & 0 deletions tests/regression/76-memleak/10-global-struct-no-ptr.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
//PARAM: --set ana.malloc.unique_address_count 1 --set ana.activated[+] memLeak
#include <stdlib.h>

typedef struct st {
int *a;
int b;
} st;

st st_nonptr;

int main(int argc, char const *argv[]) {
st_nonptr.a = malloc(sizeof(int));
st_nonptr.a = NULL;

return 0; //WARN
}
19 changes: 19 additions & 0 deletions tests/regression/76-memleak/11-global-struct-ptr.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
//PARAM: --set ana.malloc.unique_address_count 1 --set ana.activated[+] memLeak
#include <stdlib.h>

typedef struct st {
int *a;
int b;
} st;

st *st_ptr;

int main(int argc, char const *argv[]) {
st_ptr = malloc(sizeof(st));
st_ptr->a = malloc(sizeof(int));
st_ptr->a = NULL;
free(st_ptr);

// Only st_ptr->a is causing trouble here
return 0; //WARN
}
25 changes: 25 additions & 0 deletions tests/regression/76-memleak/12-global-nested-struct-ptr.c
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//PARAM: --set ana.malloc.unique_address_count 1 --set ana.activated[+] memLeak
#include <stdlib.h>

typedef struct st {
int *a;
int b;
} st;

typedef struct st2 {
st *st_ptr;
} st2;

st2 *st_var;

int main(int argc, char const *argv[]) {
st_var = malloc(sizeof(st2));
st_var->st_ptr = malloc(sizeof(st));
st_var->st_ptr->a = malloc(sizeof(int));
st_var->st_ptr->a = NULL;
free(st_var->st_ptr);
free(st_var);

// Only st_var->st_ptr->a is causing trouble here
return 0; //WARN
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
//PARAM: --set ana.malloc.unique_address_count 1 --set ana.activated[+] memLeak
#include <stdlib.h>

typedef struct st {
int *a;
int b;
} st;

typedef struct st2 {
st *st_ptr;
} st2;

st2 *st_var;

int main(int argc, char const *argv[]) {
st_var = malloc(sizeof(st2));
st_var->st_ptr = malloc(sizeof(st));
int *local_ptr = malloc(sizeof(int));
st_var->st_ptr->a = local_ptr;
local_ptr = NULL;

free(st_var->st_ptr);
free(st_var);

// local_ptr's memory is reachable through st_var->st_ptr->a
// It's leaked, because we don't call free() on it
// Hence, there should be a single warning for a memory leak, but not for unreachable memory
return 0; //WARN
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//PARAM: --set ana.malloc.unique_address_count 1 --set ana.activated[+] memLeak
#include <stdlib.h>

typedef struct st {
int *a;
int b;
} st;

typedef struct st2 {
st *st_ptr;
} st2;

st2 st_var;

int main(int argc, char const *argv[]) {
st_var.st_ptr = malloc(sizeof(st));
int *local_ptr = malloc(sizeof(int));
st_var.st_ptr->a = local_ptr;
local_ptr = NULL;
free(st_var.st_ptr);

// local_ptr's memory is reachable through st_var.st_ptr->a, but it's not freed
// Hence, there should be only a single warning for a memory leak, but not for unreachable memory
return 0; //WARN
}