On Fri, May 01, 2020 at 09:59:35PM +0200, michael-dev wrote: > 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. Right. # nft add rule ip x y ct state new,established,related,untracked # nft list ruleset nft: datatype.c:1086: datatype_set: Assertion `expr->dtype != dtype' failed. Aborted > 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. I'm going to apply this. I think it's safe to turn this into noop.
