The method `ID::AddOptionHandler()` expects to adopt a reference to
the `callback` parameter from the caller, but the caller references
the containing `Val` instance, not the `Func`.
Later, the `ID` destructor will `Unref()` the `Func`, which will
quickly underflow the reference counter. The containing `Val` however
will have references nobody will ever release (memory leak).
Zeek scripts located on separate filesystems, but sharing the same inode
number leads to scripts not being loaded. The reason is that a `ScannedFile`
is only identified by `st_ino` which is not enough to uniquely identify a
file in a system.
This problem may be hit when `ZEEKPATH` points to separate filesystems and
two script files happen have the same `st_ino` value - definitely not very
likely, but possibly very confusing when it happens.
The following test case creates two zeek scripts on separate filesystems.
As the filesystems are freshly created and of the same type, the files will
(tested a few times with xfs/ext4) have the same `st_ino` values.
#!/bin/bash
ZEEKDIR=${ZEEKDIR:-/home/awelzel/projects/zeek}
export ZEEKPATH=.:${ZEEKDIR}/build/scripts:${ZEEKDIR}/scripts
cat << EOF > hello.zeek
event zeek_init() {
print("Hello, once or twice?");
}
EOF
for i in 1 2 ; do
dd if=/dev/urandom of=img${i} count=16 bs=1M 2>/dev/null
sudo mkfs.xfs -q ./img${i}
mkdir -p mount${i}
sudo mount ./img${i} ./mount${i}
sudo cp hello.zeek ./mount${i}/hello.zeek
done
ls ./mount*/*zeek
stat -c "%n: device=%d inode=%i" ./mount*/hello.zeek
${ZEEKDIR}/build/src/zeek -b ./mount1/hello.zeek ./mount2/hello.zeek
# Cleanup
for i in 1 2 ; do
sudo umount ./mount${i}
rm -rfv ./img${i} ./mount${i}
rm -rfv hello.zeek
done
Before this patch, `Hello, once or twice?` is printed only once,
afterwards twice:
$ sh testcase.sh
[sudo] password for awelzel:
./mount1/hello.zeek ./mount2/hello.zeek
./mount1/hello.zeek: device=1794 inode=6915
./mount2/hello.zeek: device=1795 inode=6915
Hello, once or twice?
Hello, once or twice?
This is the result of (major * 10000 + minor * 100 + patch), for example
3.1.2 becomes 30102. This definition may be helpful for external code
that requires conditional compilation to support multiple Zeek
versions with differing APIs.
Minor formatting/style changes in merge.
* 'refactor_obj' of https://github.com/MaxKellermann/zeek:
Val: use C++ initializers
Val: add BroValUnion constructors
Val: reduce duplicate code by using delegating constructors
Val: remove unused default constructors and `friend` declarations
Val: remove the unnecessary BroValUnion typedef
Type: remove unnecessary enum typedefs
Type: use C++ initializers
Type: move code from BroType::BroType() to constexpr functions
Type: remove useless BroType destructor
Obj: disallow copying BroObj
Obj: use C++ initializers
Obj: make `no_location` constexpr
Minor formatting change in merge so [[noreturn]] is consistently
on same line as function declarations.
* 'noreturn' of https://github.com/MaxKellermann/zeek:
threading/MsgThread: add [[noreturn]] to InternalError()
Flare: add [[noreturn]] to bad_pipe_op()
Obj: add [[noreturn]] attributes to Internal(), bad_ref()
Reporter: add [[noreturn]] attribute to several methods
Fixes this compiler warning:
src/digest.cc: In function ‘EVP_MD_CTX* hash_init(HashAlgorithm)’:
src/digest.cc:44:26: warning: ‘md’ may be used uninitialized in this function [-Wmaybe-uninitialized]
if ( ! EVP_DigestInit_ex(c, md, NULL) )
~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~
Minor whitespace fixes during merge.
* 'smart_ptr' of https://github.com/MaxKellermann/zeek:
OpaqueVal: remove misplaced `virtual` keywords
CompHash: use class IntrusivePtr for the `type` field
IntrusivePtr: replace the "add_ref" parameter with tag structs
IntrusivePtr: remove reset(), nobody uses it
IntrusivePtr: remove ordering operators
IntrusivePtr: rename detach() to release()
IntrusivePtr: move nullptr initializer to field declaration
Updated the Ref() to happen inline with Assign() call for clarity.
* 'expr_missing_ref' of https://github.com/MaxKellermann/zeek:
Expr: add missing reference in AssignExpr::InitVal()
This method mimicks std::unique_ptr::reset(), but adds an obscure
"add_ref" parameter which is error prone. Since nobody uses this
method, and this method is all about dealing with raw pointers which
we shouldn't be doing, let's remove it.
The one reference returned by `op2->InitVal()` is given to
`aggr_r->Assign()` and returned to the caller, which may result in a
use-after-free crash bug. This patch adds the missing reference.
Closes https://github.com/zeek/zeek/issues/805
The merge commit fixes reference counting issues introduced with the
changes to ListExpr::Assign() and IndexExpr::Assign(), but then also
several other pre-existing reference counting confusions in other
Assign() implementations/calls, some which were now directly observable
via new crashing behavior, others just from a cursory code audit.
* 'memleaks' of https://github.com/MaxKellermann/zeek:
input/Manager: fix memory leak in UnrollRecordType()
OpaqueVal: fix two memory leaks in BloomFilterVal::Merge()
DbgBreakpoint: fix memory leak
DebugCmds: fix memory leak
scan.l: fix crash bug in do_atif()
Expr: fix memory leak in RecordConstructorExpr::InitVal()
Expr: fix memory leaks in AssignExpr::EvalIntoAggregate()
Expr: fix memory leaks in CondExpr::Eval()
Expr: fix several memory leaks in BoolExpr::Eval()
Expr: fix various memory leaks in Assign()
Expr: fix memory leaks in BinaryExpr::Eval()
analyzer/protocol/http: fix potential memory leak
The `Val*` return by Expr::Eval() is never freed. Note that it must
be freed after the `Field` object has been created, because the
`secondary` variable points to data inside the `Val*`.
This is really a memory leak because the Unref() call is missing. But
since this usually returns a "stock" object (`ValManager::b_true` or
`ValManager::b_false`), nothing really leaks. But eventually, the
reference counter will overflow to `INT_MAX`, leading to a crash in
bad_ref().
No code path had any cleanup code, leaking all the local references.
More weird was however the result building code: it took elements from
one of the existing vectors without referencing them, and passed them
to VectorVal::Assign() which assumes that the caller-owned reference
is now owned by that VectorVal.
Even in the successful code path, no references were freed.
Everything was wrong with this method!
