Robert Edmonds created AVRO-2458:
------------------------------------
Summary: Unsafe to call avro_value_decref() on a zero-initialized
value
Key: AVRO-2458
URL: https://issues.apache.org/jira/browse/AVRO-2458
Project: Apache Avro
Issue Type: Bug
Reporter: Robert Edmonds
Hi!
I would like to be able to use the avro-c library in a modern C code base that
uses the GCC cleanup attribute to implicitly deallocate variables when they go
out of scope. This coding style reduces or eliminates boiler plate calls to
deallocation functions in the error handling path. With fewer explicit calls to
deallocation functions, this reduces the incidence of memory leak bugs.
For avro_value_t variables, I noticed that avro_value_decref() has a compatible
function signature, so I first tried something like:
{code:java|title=avro-value-unsafe.c}
#include <avro.h>
#define _cleanup_avro_value_ \
__attribute__((cleanup(avro_value_decref)))
int
main(void)
{
_cleanup_avro_value_ avro_value_t value = {0};
/*
* Something that may or may not initialize 'value'...
*/
return 0;
}
{code}
Unfortunately, this doesn't work, because avro_value_decref() cannot handle a
zero-initialized value. It treats its parameter as if it had already been
initialized by another function in the avro-c API. So the above code crashes
due to a NULL pointer dereference:
{code:java}
==14922== Memcheck, a memory error detector
==14922== Copyright (C) 2002-2017, and GNU GPL'd, by Julian Seward et al.
==14922== Using Valgrind-3.14.0 and LibVEX; rerun with -h for copyright info
==14922== Command: ./avro-value-unsafe
==14922==
==14922== Invalid read of size 8
==14922== at 0x487BEA7: avro_value_decref (value.c:45)
==14922== by 0x10915E: main (in /tmp/avro/avro-value-unsafe)
==14922== Address 0x18 is not stack'd, malloc'd or (recently) free'd
==14922==
==14922==
==14922== Process terminating with default action of signal 11 (SIGSEGV):
dumping core
==14922== Access not within mapped region at address 0x18
==14922== at 0x487BEA7: avro_value_decref (value.c:45)
==14922== by 0x10915E: main (in /tmp/avro/avro-value-unsafe)
==14922== If you believe this happened as a result of a stack
==14922== overflow in your program's main thread (unlikely but
==14922== possible), you can try to increase the size of the
==14922== main thread stack using the --main-stacksize= flag.
==14922== The main thread stack size used in this run was 8388608.
==14922==
==14922== HEAP SUMMARY:
==14922== in use at exit: 0 bytes in 0 blocks
==14922== total heap usage: 1 allocs, 1 frees, 72,704 bytes allocated
==14922==
==14922== All heap blocks were freed -- no leaks are possible
==14922==
==14922== For counts of detected and suppressed errors, rerun with: -v
==14922== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)
{code}
[Specifically, avro_value_decref() attempts to dereference the 'iface' field of
avro_value_t objects without checking if this pointer is
non-NULL.|https://github.com/apache/avro/blob/96a7fb08e74cb7be1c63ab9c73ce0b25b1abfed1/lang/c/src/value.c#L45]
Instead, I ended up having to write a cleanup wrapper function that is safe to
call on initialized avro_value_t's (including zero-initialized avro_value_t's):
{code:java|title=avro-value-safe.c}
#include <avro.h>
static inline void
safe_avro_value_decref(avro_value_t *value)
{
if (value != NULL && value->iface != NULL) {
avro_value_decref(value);
}
}
#define _cleanup_avro_value_ \
__attribute__((cleanup(safe_avro_value_decref)))
int
main(void)
{
_cleanup_avro_value_ avro_value_t value = {0};
/*
* Something that may or may not initialize 'value'...
*/
return 0;
}
{code}
It would be nice if the deallocation functions in the avro-c API such as
avro_value_decref() were safe to call on zero-initialized objects, rather than
burdening callers with the responsibility to detect this condition. For
instance, consider by analogy the free() function in the C standard library
which is safe to call with a NULL parameter.
Thanks!
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)
