Skip to content

Commit

Permalink
fix: Avoid invalid access into random region in memory in OperatorUti…
Browse files Browse the repository at this point in the history
…ls (#12253)

Summary:
Pull Request resolved: #12253

There are 2 bugs in processEncodedFilterResults:

1. We may call bits::isBitSet(values, index) and then rows.isValid(i), this means we may be passing in an invalid index to `bits::isBitSet(values, index)`. We need to call `rows.isValid(i)` before `bits::isBitSet(values, index)`. Because index may give a bogus value if it is not valid

2. We may access an invalid value in index via `auto index = indices[i];` if i is not valid.

Fix is to check if the row is valid before processing

Reviewed By: spershin

Differential Revision: D69095863

fbshipit-source-id: 39ea8e4ced8f523f40387293848017035ac1a012
  • Loading branch information
yuandagits authored and facebook-github-bot committed Feb 4, 2025
1 parent b2cb03e commit 48d1bbf
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 1 deletion.
5 changes: 4 additions & 1 deletion velox/exec/OperatorUtils.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -231,9 +231,12 @@ vector_size_t processEncodedFilterResults(
auto* rawSelectedBits = filterEvalCtx.getRawSelectedBits(size, pool);
memset(rawSelectedBits, 0, bits::nbytes(size));
for (int32_t i = 0; i < size; ++i) {
if (!rows.isValid(i)) {
continue;
}
auto index = indices[i];
if ((!nulls || !bits::isBitNull(nulls, i)) &&
bits::isBitSet(values, index) && rows.isValid(i)) {
bits::isBitSet(values, index)) {
rawSelected[passed++] = i;
bits::setBit(rawSelectedBits, i);
}
Expand Down
12 changes: 12 additions & 0 deletions velox/exec/tests/OperatorUtilsTest.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,18 @@ class OperatorUtilsTest : public OperatorTestBase {
std::unique_ptr<DriverCtx> driverCtx_;
};

TEST_F(OperatorUtilsTest, processFilterResults) {
auto filteredResults = makeArrayVector<int64_t>({{1}});
SelectivityVector filterRows(1);
filterRows.setValid(0, false);
filterRows.updateBounds();
exec::FilterEvalCtx filterEvalCtx;
EXPECT_EQ(
exec::processFilterResults(
filteredResults, filterRows, filterEvalCtx, pool_.get()),
0);
}

TEST_F(OperatorUtilsTest, wrapChildConstant) {
auto constant = makeConstant(11, 1'000);

Expand Down

0 comments on commit 48d1bbf

Please sign in to comment.