OpenE2K/gcc
2
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

analyzer: fixes to realloc-handling [PR104369]

Browse Source 
This patch fixes various issues with how -fanalyzer handles "realloc"
seen when debugging PR analyzer/104369.

Previously it wasn't correctly copying over the contents of the old
buffer for the success-with-move case, leading to false
-Wanalyzer-use-of-uninitialized-value diagnostics.

I also noticed that -fanalyzer failed to properly handle "realloc" for
cases where the ptr's region had unknown dynamic extents, and an ICE
for the case where a tainted value is used as a realloc size argument.

This patch fixes these issues, including the false uninit diagnostics
seen in PR analyzer/104369.

gcc/analyzer/ChangeLog:
	PR analyzer/104369
	* engine.cc (exploded_graph::process_node): Use the node for any
	diagnostics, avoiding ICE if a bifurcation update adds a
	saved_diagnostic, such as for a tainted realloc size.
	* region-model-impl-calls.cc
	(region_model::impl_call_realloc::success_no_move::update_model):
	Require the old pointer to be non-NULL to be able successfully
	grow in place.  Use model->deref_rvalue rather than maybe_get_region
	to support the old pointer being symbolic.
	(region_model::impl_call_realloc::success_with_move::update_model):
	Likewise.  Add a constraint that the new pointer != the old pointer.
	Use a sized_region when setting the value of the new region.
	Handle the case where we don't know the dynamic size of the old
	region by marking the new region as unknown.
	* sm-taint.cc (tainted_allocation_size::tainted_allocation_size):
	Update assertion to also allow for MEMSPACE_UNKNOWN.
	(tainted_allocation_size::emit): Likewise.
	(region_model::check_dynamic_size_for_taint): Likewise.

gcc/testsuite/ChangeLog:
	PR analyzer/104369
	* gcc.dg/analyzer/pr104369-1.c: New test.
	* gcc.dg/analyzer/pr104369-2.c: New test.
	* gcc.dg/analyzer/realloc-3.c: New test.
	* gcc.dg/analyzer/realloc-4.c: New test.
	* gcc.dg/analyzer/taint-realloc.c: New test.

Signed-off-by: David Malcolm <dmalcolm@redhat.com>
This commit is contained in:
David Malcolm 2022-02-02 16:39:12 -05:00
parent 23b2cb628e
commit 3ef328c293
8 changed files with 392 additions and 7 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
gcc/analyzer/engine.cc
View File

@ -3799,7 +3799,7 @@ exploded_graph::process_node (exploded_node *node)
/* Apply edge_info to state. */
impl_region_model_context
bifurcation_ctxt (*this,
NULL, // enode_for_diag
node, // enode_for_diag
&path_ctxt.get_state_at_bifurcation (),
&bifurcated_new_state,
NULL, // uncertainty_t *uncertainty

33
gcc/analyzer/region-model-impl-calls.cc
View File

@ -651,7 +651,18 @@ region_model::impl_call_realloc (const call_details &cd)
const call_details cd (get_call_details (model, ctxt));
const svalue *ptr_sval = cd.get_arg_svalue (0);
const svalue *size_sval = cd.get_arg_svalue (1);
if (const region *buffer_reg = ptr_sval->maybe_get_region ())
/* We can only grow in place with a non-NULL pointer. */
{
const svalue *null_ptr
= model->m_mgr->get_or_create_int_cst (ptr_sval->get_type (), 0);
if (!model->add_constraint (ptr_sval, NE_EXPR, null_ptr,
cd.get_ctxt ()))
return false;
}
if (const region *buffer_reg = model->deref_rvalue (ptr_sval, NULL_TREE,
ctxt))
if (compat_types_p (size_sval->get_type (), size_type_node))
model->set_dynamic_extents (buffer_reg, size_sval, ctxt);
if (cd.get_lhs_region ())
@ -696,10 +707,15 @@ region_model::impl_call_realloc (const call_details &cd)
= model->create_region_for_heap_alloc (new_size_sval, ctxt);
const svalue *new_ptr_sval
= model->m_mgr->get_ptr_svalue (cd.get_lhs_type (), new_reg);
if (!model->add_constraint (new_ptr_sval, NE_EXPR, old_ptr_sval,
cd.get_ctxt ()))
return false;
if (cd.get_lhs_type ())
cd.maybe_set_lhs (new_ptr_sval);
if (const region *freed_reg = old_ptr_sval->maybe_get_region ())
if (const region *freed_reg = model->deref_rvalue (old_ptr_sval,
NULL_TREE, ctxt))
{
/* Copy the data. */
const svalue *old_size_sval = model->get_dynamic_extents (freed_reg);
@ -710,7 +726,18 @@ region_model::impl_call_realloc (const call_details &cd)
old_size_sval);
const svalue *buffer_content_sval
= model->get_store_value (sized_old_reg, cd.get_ctxt ());
model->set_value (new_reg, buffer_content_sval, cd.get_ctxt ());
const region *sized_new_reg
= model->m_mgr->get_sized_region (new_reg, NULL,
old_size_sval);
model->set_value (sized_new_reg, buffer_content_sval,
cd.get_ctxt ());
}
else
{
/* We don't know how big the old region was;
mark the new region as having been touched to avoid uninit
issues. */
model->mark_region_as_unknown (new_reg, cd.get_uncertainty ());
}
/* Free the old region, so that pointers to the old buffer become

12
gcc/analyzer/sm-taint.cc
View File

@ -496,7 +496,9 @@ public:
: taint_diagnostic (sm, arg, has_bounds),
m_mem_space (mem_space)
{
gcc_assert (mem_space == MEMSPACE_STACK || mem_space == MEMSPACE_HEAP);
gcc_assert (mem_space == MEMSPACE_STACK
|| mem_space == MEMSPACE_HEAP
|| mem_space == MEMSPACE_UNKNOWN);
}
const char *get_kind () const FINAL OVERRIDE
@ -509,7 +511,9 @@ public:
diagnostic_metadata m;
/* "CWE-789: Memory Allocation with Excessive Size Value". */
m.add_cwe (789);
gcc_assert (m_mem_space == MEMSPACE_STACK || m_mem_space == MEMSPACE_HEAP);
gcc_assert (m_mem_space == MEMSPACE_STACK
|| m_mem_space == MEMSPACE_HEAP
|| m_mem_space == MEMSPACE_UNKNOWN);
// TODO: make use of m_mem_space
if (m_arg)
switch (m_has_bounds)
@ -1051,7 +1055,9 @@ region_model::check_dynamic_size_for_taint (enum memory_space mem_space,
const svalue *size_in_bytes,
region_model_context *ctxt) const
{
gcc_assert (mem_space == MEMSPACE_STACK || mem_space == MEMSPACE_HEAP);
gcc_assert (mem_space == MEMSPACE_STACK
|| mem_space == MEMSPACE_HEAP
|| mem_space == MEMSPACE_UNKNOWN);
gcc_assert (size_in_bytes);
gcc_assert (ctxt);

86
gcc/testsuite/gcc.dg/analyzer/pr104369-1.c Normal file
View File

@ -0,0 +1,86 @@
/* { dg-additional-options "--param analyzer-max-enodes-per-program-point=10" } */
// TODO: remove need for this option
typedef __SIZE_TYPE__ size_t;
#define NULL ((void *)0)
#define POLLIN 0x001
typedef struct {
unsigned long int __val[(1024 / (8 * sizeof(unsigned long int)))];
} __sigset_t;
typedef unsigned int socklen_t;
struct timespec {
/* [...snip...] */
};
typedef unsigned long int nfds_t;
struct pollfd {
int fd;
short int events;
short int revents;
};
extern int ppoll(struct pollfd *__fds, nfds_t __nfds,
const struct timespec *__timeout, const __sigset_t *__ss);
struct sockaddr_un {
/* [...snip...] */
char sun_path[108];
};
typedef union {
/* [...snip...] */
struct sockaddr_un *__restrict __sockaddr_un__;
/* [...snip...] */
} __SOCKADDR_ARG __attribute__((transparent_union));
extern int accept(int __fd, __SOCKADDR_ARG __addr,
socklen_t *__restrict __addr_len);
extern void *calloc(size_t __nmemb, size_t __size)
__attribute__((__nothrow__, __leaf__))
__attribute__((__malloc__))
__attribute__((__alloc_size__(1, 2)));
extern void *realloc(void *__ptr, size_t __size)
__attribute__((__nothrow__, __leaf__))
__attribute__((__warn_unused_result__))
__attribute__((__alloc_size__(2)));
extern void exit(int __status)
__attribute__((__nothrow__, __leaf__))
__attribute__((__noreturn__));
int main() {
int rc;
int nsockets = 1;
struct pollfd *pollfds, *newpollfds;
struct sockaddr_un remote;
socklen_t len = sizeof(remote);
pollfds = calloc(1, sizeof(struct pollfd));
if (!pollfds) {
exit(1);
}
while (1) {
rc = ppoll(pollfds, nsockets, NULL, NULL);
if (rc < 0) {
continue;
}
if (pollfds[0].revents & POLLIN) {
nsockets++;
newpollfds = realloc(pollfds, nsockets * sizeof(*pollfds));
if (!newpollfds) {
exit(1);
}
pollfds = newpollfds;
pollfds[nsockets - 1].fd = accept(pollfds[0].fd, &remote, &len);
}
}
return 0;
}

79
gcc/testsuite/gcc.dg/analyzer/pr104369-2.c Normal file
View File

@ -0,0 +1,79 @@
typedef __SIZE_TYPE__ size_t;
#define NULL ((void *)0)
#define POLLIN 0x001
typedef struct {
unsigned long int __val[(1024 / (8 * sizeof(unsigned long int)))];
} __sigset_t;
typedef unsigned int socklen_t;
struct timespec {
/* [...snip...] */
};
typedef unsigned long int nfds_t;
struct pollfd {
int fd;
short int events;
short int revents;
};
extern int ppoll(struct pollfd *__fds, nfds_t __nfds,
const struct timespec *__timeout, const __sigset_t *__ss);
struct sockaddr_un {
/* [...snip...] */
char sun_path[108];
};
typedef union {
/* [...snip...] */
struct sockaddr_un *__restrict __sockaddr_un__;
/* [...snip...] */
} __SOCKADDR_ARG __attribute__((transparent_union));
extern int accept(int __fd, __SOCKADDR_ARG __addr,
socklen_t *__restrict __addr_len);
extern void *calloc(size_t __nmemb, size_t __size)
__attribute__((__nothrow__, __leaf__))
__attribute__((__malloc__))
__attribute__((__alloc_size__(1, 2)));
extern void *realloc(void *__ptr, size_t __size)
__attribute__((__nothrow__, __leaf__))
__attribute__((__warn_unused_result__))
__attribute__((__alloc_size__(2)));
extern void exit(int __status)
__attribute__((__nothrow__, __leaf__))
__attribute__((__noreturn__));
int main() {
int rc;
int nsockets = 1;
struct pollfd *pollfds, *newpollfds;
struct sockaddr_un remote;
socklen_t len = sizeof(remote);
pollfds = calloc(1, sizeof(struct pollfd));
if (!pollfds) {
exit(1);
}
rc = ppoll(pollfds, nsockets, NULL, NULL);
if (rc < 0) {
exit(2);
}
nsockets++;
newpollfds = realloc(pollfds, nsockets * sizeof(*pollfds));
if (!newpollfds) {
exit(3);
}
pollfds = newpollfds;
pollfds[nsockets - 1].fd = accept(pollfds[0].fd, &remote, &len);
exit(4);
}

81
gcc/testsuite/gcc.dg/analyzer/realloc-3.c Normal file
View File

@ -0,0 +1,81 @@
#include "analyzer-decls.h"
typedef __SIZE_TYPE__ size_t;
#define NULL ((void *)0)
extern void *calloc (size_t __nmemb, size_t __size)
__attribute__ ((__nothrow__ , __leaf__))
__attribute__ ((__malloc__))
__attribute__ ((__alloc_size__ (1, 2))) ;
extern void *malloc (size_t __size)
__attribute__ ((__nothrow__ , __leaf__))
__attribute__ ((__malloc__))
__attribute__ ((__alloc_size__ (1)));
extern void *realloc (void *__ptr, size_t __size)
__attribute__ ((__nothrow__ , __leaf__))
__attribute__ ((__warn_unused_result__))
__attribute__ ((__alloc_size__ (2)));
extern void free (void *__ptr)
__attribute__ ((__nothrow__ , __leaf__));
/* realloc of calloc buffer. */
char *test_8 (size_t sz)
{
char *p, *q;
p = calloc (1, 3);
if (!p)
return NULL;
__analyzer_dump_capacity (p); /* { dg-warning "capacity: '\\(\[^\n\r\]*\\)3'" } */
__analyzer_eval (p[0] == 0); /* { dg-warning "TRUE" } */
__analyzer_eval (p[1] == 0); /* { dg-warning "TRUE" } */
__analyzer_eval (p[2] == 0); /* { dg-warning "TRUE" } */
q = realloc (p, 6);
/* We should have 3 nodes, corresponding to "failure",
"success without moving", and "success with moving". */
__analyzer_dump_exploded_nodes (0); /* { dg-warning "3 processed enodes" } */
if (q)
{
__analyzer_dump_capacity (q); /* { dg-warning "capacity: '\\(\[^\n\r\]*\\)6'" } */
q[3] = 'd';
q[4] = 'e';
q[5] = 'f';
if (q == p)
{
/* "realloc" success, growing the buffer in-place. */
__analyzer_eval (p[0] == 0); /* { dg-warning "TRUE" } */
__analyzer_eval (p[1] == 0); /* { dg-warning "TRUE" } */
__analyzer_eval (p[2] == 0); /* { dg-warning "TRUE" } */
}
else
{
/* "realloc" success, moving the buffer (and thus freeing "p"). */
__analyzer_eval (q[0] == 0); /* { dg-warning "TRUE" } */
__analyzer_eval (q[1] == 0); /* { dg-warning "TRUE" } */
__analyzer_eval (q[2] == 0); /* { dg-warning "TRUE" } */
__analyzer_eval (p[0] == 'a'); /* { dg-warning "UNKNOWN" "unknown" } */
/* { dg-warning "use after 'free' of 'p'" "use after free" { target *-*-* } .-1 } */
}
__analyzer_eval (q[3] == 'd'); /* { dg-warning "TRUE" } */
__analyzer_eval (q[4] == 'e'); /* { dg-warning "TRUE" } */
__analyzer_eval (q[5] == 'f'); /* { dg-warning "TRUE" } */
}
else
{
/* "realloc" failure. p should be unchanged. */
__analyzer_dump_capacity (p); /* { dg-warning "capacity: '\\(\[^\n\r\]*\\)3'" } */
__analyzer_eval (p[0] == 0); /* { dg-warning "TRUE" } */
__analyzer_eval (p[1] == 0); /* { dg-warning "TRUE" } */
__analyzer_eval (p[2] == 0); /* { dg-warning "TRUE" } */
return p;
}
return q;
}

85
gcc/testsuite/gcc.dg/analyzer/realloc-4.c Normal file
View File

@ -0,0 +1,85 @@
#include "analyzer-decls.h"
typedef __SIZE_TYPE__ size_t;
#define NULL ((void *)0)
extern void *calloc (size_t __nmemb, size_t __size)
__attribute__ ((__nothrow__ , __leaf__))
__attribute__ ((__malloc__))
__attribute__ ((__alloc_size__ (1, 2))) ;
extern void *malloc (size_t __size)
__attribute__ ((__nothrow__ , __leaf__))
__attribute__ ((__malloc__))
__attribute__ ((__alloc_size__ (1)));
extern void *realloc (void *__ptr, size_t __size)
__attribute__ ((__nothrow__ , __leaf__))
__attribute__ ((__warn_unused_result__))
__attribute__ ((__alloc_size__ (2)));
extern void free (void *__ptr)
__attribute__ ((__nothrow__ , __leaf__));
/* realloc where we don't know the original size of the region. */
char *test_8 (char *p, size_t sz)
{
char *q;
__analyzer_dump_capacity (p); /* { dg-warning "capacity: 'UNKNOWN\\(\[^\n\r\]*\\)'" } */
p[0] = 'a';
p[1] = 'b';
p[2] = 'c';
__analyzer_eval (p[0] == 'a'); /* { dg-warning "TRUE" } */
__analyzer_eval (p[1] == 'b'); /* { dg-warning "TRUE" } */
__analyzer_eval (p[2] == 'c'); /* { dg-warning "TRUE" } */
q = realloc (p, 6);
/* We should have 3 nodes, corresponding to "failure",
"success without moving", and "success with moving". */
__analyzer_dump_exploded_nodes (0); /* { dg-warning "3 processed enodes" } */
if (q)
{
q[3] = 'd';
q[4] = 'e';
q[5] = 'f';
if (q == p)
{
/* "realloc" success, growing the buffer in-place. */
__analyzer_dump_exploded_nodes (0); /* { dg-warning "1 processed enode" } */
__analyzer_dump_capacity (q); /* { dg-warning "capacity: '\\(\[^\n\r\]*\\)6'" } */
__analyzer_eval (q[0] == 'a'); /* { dg-warning "TRUE" } */
__analyzer_eval (q[1] == 'b'); /* { dg-warning "TRUE" } */
__analyzer_eval (q[2] == 'c'); /* { dg-warning "TRUE" } */
}
else
{
/* "realloc" success, moving the buffer (and thus freeing "p"). */
__analyzer_dump_exploded_nodes (0); /* { dg-warning "1 processed enode" } */
__analyzer_dump_capacity (q); /* { dg-warning "capacity: '\\(\[^\n\r\]*\\)6'" } */
/* We don't know how much of the buffer is copied. */
__analyzer_eval (q[0] == 'a'); /* { dg-warning "UNKNOWN" } */
__analyzer_eval (q[1] == 'b'); /* { dg-warning "UNKNOWN" } */
__analyzer_eval (q[2] == 'c'); /* { dg-warning "UNKNOWN" } */
__analyzer_eval (p[0] == 'a'); /* { dg-warning "UNKNOWN" "unknown" } */
/* { dg-warning "use after 'free' of 'p'" "use after free" { target *-*-* } .-1 } */
}
__analyzer_eval (q[3] == 'd'); /* { dg-warning "TRUE" } */
__analyzer_eval (q[4] == 'e'); /* { dg-warning "TRUE" } */
__analyzer_eval (q[5] == 'f'); /* { dg-warning "TRUE" } */
}
else
{
/* "realloc" failure. p should be unchanged. */
__analyzer_dump_exploded_nodes (0); /* { dg-warning "1 processed enode" } */
__analyzer_dump_capacity (q); /* { dg-warning "capacity: 'UNKNOWN\\(\[^\n\r\]*\\)'" } */
__analyzer_eval (p[0] == 'a'); /* { dg-warning "TRUE" } */
__analyzer_eval (p[1] == 'b'); /* { dg-warning "TRUE" } */
__analyzer_eval (p[2] == 'c'); /* { dg-warning "TRUE" } */
return p;
}
return q;
}

21
gcc/testsuite/gcc.dg/analyzer/taint-realloc.c Normal file
View File

@ -0,0 +1,21 @@
// TODO: remove need for this option:
/* { dg-additional-options "-fanalyzer-checker=taint" } */
#include "analyzer-decls.h"
#include <stdio.h>
#include <stdlib.h>
#include <string.h>
/* realloc with tainted size. */
void *p;
void __attribute__((tainted_args))
test_1 (size_t sz) /* { dg-message "\\(1\\) function 'test_1' marked with '__attribute__\\(\\(tainted_args\\)\\)'" } */
{
void *q;
__analyzer_dump_state ("taint", sz); /* { dg-warning "state: 'tainted'" } */
q = realloc (p, sz); /* { dg-warning "use of attacker-controlled value 'sz' as allocation size without upper-bounds checking" } */
}