Aaron Jones [Mon, 28 Feb 2022 23:25:28 +0000 (23:25 +0000)]
patricia: add a mowgli_patricia_clear() function
This does the same as mowgli_patricia_destroy(), but without actually
destroying the trie itself. Useful when you need to clear all of the
elements in a trie without losing the trie too.
You cannot always use MOWGLI_PATRICIA_FOREACH to do this, as it
returns the items on each iteration, not their key. If the item
doesn't contain the key, this is not useful.
Redefine mowgli_patricia_destroy() in terms of the new function.
Nicole Kleinhoff [Tue, 23 Feb 2021 00:34:16 +0000 (00:34 +0000)]
core/heap.disabled: rip out HEAP_DEBUG code
There's little point in basically just reporting the equivalent of
malloc()/free() each time it happens; there isn't really anything to
debug here.
This also fixes a bug introduced in 101e46d8ac; the HEAP_DEBUG code had
not been properly adjusted and still referenced a variable removed in
that commit, thus breaking compilation with HEAP_DEBUG enabled.
Nicole Kleinhoff [Mon, 22 Feb 2021 23:30:45 +0000 (23:30 +0000)]
core/heap.disabled: remove tracking of allocations
We used to keep track of allocated pointers to warn about them if the
heap was destroyed without all pointers being freed first. However, this
means we keep a list of pointers around until the heap is destroyed,
which for certain heaps may never happen; this will cause tools such as
Valgrind or LeakSanitizer to (correctly) see a valid pointer remaining
to the allocated memory and thus not detect a leak, even if any actual
*useful* pointers were lost.
These tools are more general and can also provide backtraces indicating
where the leaked memory was allocated, hence are strictly superior to
the check we did in mowgli_heap_destroy. Removing the remaining
bookkeeping allows them to work on mowgli_heap-allocated memory.
This essentially turns mowgli_heap_t and the related functions into a
wrapper around the configured allocate/deallocate functions except
always passing a fixed size. Since this is the code used when users
specifically requested to build without the heap allocator, that seems
like it shouldn't violate any assumptions.
Nicole Kleinhoff [Mon, 22 Feb 2021 22:56:32 +0000 (22:56 +0000)]
eventloop: delay freeing pollables until the current iteration finishes
mowgli_pollable_destroy would free the pollable, however if called from
within a handler function invoked by the event loop, the event loop
might still have a pointer to it waiting to be processed, causing a
use-after-free.
Instead, keep track of whether we are currently processing events (i.e.
calling a pollops function that will process events); if we are doing
so, mark the pollable as removed and add it to a list of to-be-freed
pollables.
Individual implementations check the removed flag and ignore pollables
with it set; the general-case abstraction is responsible for keeping
track of whether we are currently in an event processing function and
cleaning up the list of pollables after each iteration.
For users of libmowgli, there is no change to API nor ABI as long as
programs use mowgli_{eventloop,pollable}_create to allocate the involved
structures.
Aaron Jones [Mon, 21 Jan 2019 10:37:57 +0000 (10:37 +0000)]
core/heap.enabled: use size_t consistently
The previous code was using `unsigned int'. This could be truncated on
a 64-bit system with many allocations.
Fortunately this implementation is entirely internal, and so changing
the data type does not break the ABI; the function declarations were
already using the correct type.
The heap object stores its allocator for its entire lifetime, so
there's no need to call mowgli_alloc()/mowgli_free(), which also
stores the allocator tag behind the returned pointer.
This will (sometimes) reduce the memory consumption of every
allocation by a pointer and is slightly more efficient.
Also add some debugging assertions to validate the integrity of
the given/chosen allocator.
Aaron Jones [Sun, 20 Jan 2019 17:38:44 +0000 (17:38 +0000)]
Build System & core/heap: add method to disable heap allocator
The heap allocator was found to be hiding a use-after-free error
in an unrelated program (not Atheme IRC Services).
Add a configure option to make mowgli.heap just wrap around
mowgli.alloc directly. This allows hardened memory allocators to
be plugged into the latter, which affect all individual object
allocations made by the former.
The disabled implementation also features more debug logging.
The latter is easier to read, and if the type name of the
variable is changed in the future, the type name inside the
parameter does not need to be changed too.
Also, sizeof is not a function, so it doesn't need parentheses
when used in this manner. It did previously because the type name
contained spaces (e.g. "struct mowgli_foo"), but we're using the
variable name now, which can't contain spaces.
Aaron Jones [Fri, 18 Jan 2019 03:25:14 +0000 (03:25 +0000)]
core/bootstrap_internal.h: add missing header file to repository
git(1) keeps complaining that this file is being ignored by
.gitignore, which does not appear to be the case. Force add
it so that people can check out the repo and build it.
Aaron Jones [Fri, 18 Jan 2019 03:16:40 +0000 (03:16 +0000)]
container/dictionary.[ch]: fix numerous small issues
The 4th diagnostic in particular is interesting because it is
located in unreachable code, so simply remove the entire
conditional block which is always false.
The 5th diagnostic is useful: it illuminated a missing function
declaration in the corresponding header, which was added.
Fixes compiler diagnostics:
dictionary.c:236:1: warning: no previous prototype for function
'mowgli_dictionary_retune' [-Wmissing-prototypes]
dictionary.c:335:1: warning: no previous prototype for function
'mowgli_dictionary_link' [-Wmissing-prototypes]
dictionary.c:414:1: warning: no previous prototype for function
'mowgli_dictionary_unlink_root' [-Wmissing-prototypes]
dictionary.c:758:78: warning: cast from 'const void *' to 'void *'
drops const qualifier [-Wcast-qual]
dictionary.c:846:1: warning: no previous prototype for function
'mowgli_dictionary_size' [-Wmissing-prototypes]
Aaron Jones [Tue, 15 Jan 2019 14:34:10 +0000 (14:34 +0000)]
container/patricia: mowgli_patricia_create(): simplify in other terms
The Clang Static Analyzer identified that much of the code in
mowgli_patricia_create() was also present in the function beneath it,
mowgli_patricia_create_named().
Aaron Jones [Tue, 15 Jan 2019 14:33:00 +0000 (14:33 +0000)]
container/dictionary: mowgli_dictionary_create(): simplify in other terms
The Clang Static Analyzer identified that much of the code in
mowgli_dictionary_create() was also present in the function beneath it,
mowgli_dictionary_create_named().
Rewrite the former in terms of the latter.
Also fixes a bug where strdup(3) was used instead of mowgli_strdup().
Aaron Jones [Tue, 15 Jan 2019 14:11:27 +0000 (14:11 +0000)]
container/list: mowgli_list_concat(): fix possible NULL dereference
When concatenating a list onto an empty list, the code would dereference
the empty list's "tail" member even after checking that it is not NULL
(but not within this check, hence the problem). Move the dereference to
inside the conditional check block.
Also ensure that if the list being concatenated onto is empty, that its
"head" member is set appropriately too.
The former issue was identified by the Clang Static Analyzer.
Finally remove an "a = b = ..." idiom that trashes readability.
Aaron Jones [Tue, 15 Jan 2019 13:40:31 +0000 (13:40 +0000)]
All Source Files: Lots of header/macro/function cleanups
- Consistently use 2 spaces for preprocessor directive indentation
- Ensure all header files can successfully compile stand-alone
- Rearrange inclusions in alphabetical order; now that each header
can build by itself, it doesn't matter what order you use them in
- Replace compiler attributes macro header wholesale with the one from
Atheme IRC Services
- Add lots of missing function attributes where applicable
Memory allocation attributes in particular will be useful for
static analysis; but you can't just add the malloc attribute to
every function returning a newly-allocated pointer. The malloc
attribute is used to declare functions that return newly-allocated
memory, and if that return value is a pointer to a newly-allocated
structure, and some of its members point to memory that existed
before the function was called, then it is not entirely newly-
allocated memory, and the optimizer might generate broken code
with the faulty assumption given to it. So, a careful audit was
required to identify functions that allocated memory without setting
pointers in the resulting structures to point to things that existed
before the allocating function was called.
- Remove unnecessary includes from source files
- Fix 2 broken parameter-less function prototypes
- Avoid using "#elif" preprocessor directive where it does not result
in uglier output
Aaron Jones [Tue, 15 Jan 2019 10:12:22 +0000 (10:12 +0000)]
All Header Files: Use consistently-named & commented inclusion guards
This names the include guards after the file path and comments the
endif at the bottom of the files too. It also instructs autoconf to
generate inclusion guards for the autoconf.h file that it writes out.
This avoids the include guards being named with reserved identifiers.
Identifiers that start with 2 underscores, or 1 underscore and then a
capital letter, should only be declared by system headers.
Aaron Jones [Tue, 6 Mar 2018 21:05:08 +0000 (21:05 +0000)]
src/libmowgli/dns/evloop_res.c: fix epoll_ctl(2) -EBADF among others
mowgli_dns_evloop_destroy() calls mowgli_vio_close(), which closes the
file descriptor. It then calls mowgli_vio_destroy(), which frees it from
the event loop, and then closes the file descriptor if it is not closed
already.
The problem is that closing a file descriptor removes it from any epoll
sets automatically, so the order of operations is incorrect and results
in:
Aaron Jones [Tue, 6 Mar 2018 19:51:04 +0000 (19:51 +0000)]
src/libmowgli/dns/evloop_res.c: fix memory leak
mowgli_dns_evloop_init() [called by mowgli_dns_create()] allocates
a random number generator and assigns it to `dns->state->rand', but
mowgli_dns_evloop_destroy() [called by mowgli_dns_destroy()] does
not free this object.
This was found while running Atheme IRC Services (atheme/#621) under
Valgrind.
Aaron Jones [Mon, 26 Feb 2018 04:24:01 +0000 (04:24 +0000)]
src/libmowgli/dns/evloop_res.c: fix erroneous invocation of memcpy(3)
mowgli_dns_reslist_t.addr is a `struct sockaddr_storage', but
mowgli_dns_reply_t.addr is a `struct mowgli_vio_sockaddr',
which has both `struct sockaddr_storage' and `socklen_t'
members.
This makes mowgli_dns_reply_t.addr larger than
mowgli_dns_reslist_t.addr, but the size of the former is used
for the memory copy operation. This will result in an out-of-
bounds memory access while reading the smaller structure.
Explicitly copy to the sockaddr_storage structure within, and
use its size instead.
This was found by the Clang static analyzer while performing
an analysis on Atheme IRC Services.
Aaron Jones [Mon, 26 Feb 2018 04:06:58 +0000 (04:06 +0000)]
src/libmowgli/base/argstack.c: remove erroneous invocation of va_end(3)
The stdarg(3) manpage states:
va_end()
Each invocation of va_start() must be matched by a cor‐
responding invocation of va_end() in the *same function*.
After the call va_end(ap) the variable ap is *undefined*.
Multiple traversals of the list, each bracketed by
va_start() and va_end() are possible. va_end() may be
a macro or a function.
(Emphasis mine)
However, va_start(3) was not called in this function, and
callers of mowgli_argstack_create_from_va_list() also call
va_end(3) after it returns. This means both that the varargs
will always be cleaned up, and if the function ran into an
error, it would erroneously clean it up and then the caller
would be invoking va_end(3) on an uninitialized list.
This was found by the Clang static analyzer while performing
an analysis on Atheme IRC Services.
Aaron Jones [Tue, 5 Sep 2017 04:52:26 +0000 (04:52 +0000)]
OpenSSL: Several minor fixups
- Respect --with-openssl=no and skip tests for building against it
- Test whether openssl/ec.h exists and conditionally build with that
cf. https://bugs.gentoo.org/show_bug.cgi?id=629856
Aaron Jones [Mon, 16 Nov 2015 20:22:08 +0000 (20:22 +0000)]
Seriously rework the OpenSSL context code
* Always negotiate the highest mutually supported protocol version
* Use the new TLS_{client,server}_method() APIs from OpenSSL 1.1.0
* Explicitly disable SSLv2 and SSLv3 if so available
* Support ECDH (either prime256v1 (< 1.0.2) or automatic (>= 1.0.2))
Aaron Jones [Thu, 15 Jan 2015 07:05:25 +0000 (07:05 +0000)]
Fix IPv6 reverse DNS lookups (address to name conversion)
The bug here is that it reverses the octets and appends a period
after each nibble, but then prepends a period before ip6.arpa.
This results in a query with 2 consecutive periods, which then
goes on to fail query validation and so the query is never sent.
I have no idea how this failed basic quality testing especially
since there is an example program that uses this functionality
that was apparently not even extended to try IPv6.
If a timer is added while the deadline is not determined yet, the new
deadline only takes the new timer into account. This prevents execution
of other timers that should be run sooner.