halide / Halide

a language for fast, portable data-parallel computation
https://halide-lang.org
Other
5.91k stars 1.07k forks source link

Fix heap-use-after-free error in find_best_fit() #8483

Closed steven-johnson closed 1 day ago

steven-johnson commented 2 days ago

Fixes #8482

steven-johnson commented 1 day ago

Note that this reveals a fundamental danger of the as_const_int() and friends API (i.e. if the Expr being looked at is a temporary)...

steven-johnson commented 1 day ago

From a quick skim of the source, I don't see any other obvious misuse of these calls that seem to be at-risk, but it would be easy to misuse them inadvertently. Not sure if we want to revisit this API somehow to minimize risk.

abadams commented 1 day ago

Could we add a as_const_int(Expr &&) that just contains a static_assert to stop you from calling it on rvals? We'd still have the problem below, but it would at least prevent one source of problems.

const uint64_t *i = nullptr;
{ 
  Expr e = some temporary expr
  i = as_const_int(e);
}
if (i && *i == 5) { // use-after-free
  ...
}
abadams commented 1 day ago

Actually the better fix is making as_const_int return a std::optional<int64_t>. IIUC the syntax for using it is even the same because they made it act like a pointer (if (i & *i == 5))