Am 01.05.2020 21:27, schrieb Pablo Neira Ayuso:
On Fri, May 01, 2020 at 05:48:18PM +0200, Michael Braun wrote:
+ if (dtype == expr->dtype)
+ return; // do not free dtype before incrementing refcnt again
The problem you describe (use-after-free) happens in this case, right?
The problem is more likely due to concat_expr_parse_udata not calling
datatype_get,
because otherwise datatype_get would be in the backtrace of ASAN.
datatype_set(expr, expr->dtype);
Or am I missing anything?
But while debugging the above output, I added assert(dtype !=
expr->dtype) here
and that crashed. So I'm sure something like this is happening.
And the whole thing was nasty to debug, so I added this one just be sure
it does not happen again.
As ASAN should hit on datatype_get incrementing refcnt if datatype_free
had actually freed it,
assert was probaby not seeing an DTYPE_F_ALLOC instance.
But I dig not deeper here, as I felt this return is safe to add.
datatype_free(expr->dtype);
expr->dtype = datatype_get(dtype);
}
diff --git a/src/expression.c b/src/expression.c
index 6605beb3..a6bde70f 100644
--- a/src/expression.c
+++ b/src/expression.c
@@ -955,7 +955,7 @@ static struct expr *concat_expr_parse_udata(const
struct nftnl_udata *attr)
if (!dtype)
goto err_free;
- concat_expr->dtype = dtype;
+ concat_expr->dtype = datatype_get(dtype);
This is also good indeed.
This is what caused the ASAN output above to go away.
Regards,
M. Braun
