From ee2439b5f6d8736ca72a289b9fe0851321039504 Mon Sep 17 00:00:00 2001 From: Tristan Su Date: Mon, 9 Nov 2020 14:01:55 +0800 Subject: [PATCH] fix(tsi): close series id iterator after merging This use-after-free bug may lead to segfault. The iterators that have reference to the underlying index files were closed too early while the bitmaps were still used afterwards. If a compaction occurs concurrently and removes the index files, it would result in accessing unmap'd memory address. --- CHANGELOG.md | 1 + tsdb/index.go | 13 +++++++++---- 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fd431f63fb4..9bc163f1f34 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -86,6 +86,7 @@ RPM packages, which has been left unchanged. 1. [20702](https://github.com/influxdata/influxdb/pull/20702): Fix loading config when `INFLUXD_CONFIG_PATH` points to a directory with `.` in its name. 1. [20678](https://github.com/influxdata/influxdb/pull/20678): Fix infinite loop in Flux parser caused by invalid array expressions. 1. [20360](https://github.com/influxdata/influxdb/pull/20360): Update API spec to document Flux dictionary features. +1. [19936](https://github.com/influxdata/influxdb/pull/19936): Close series id iterator after merging ## v2.0.3 [2020-12-14] diff --git a/tsdb/index.go b/tsdb/index.go index f5c5f823b49..dc8ca6ae320 100644 --- a/tsdb/index.go +++ b/tsdb/index.go @@ -557,9 +557,11 @@ func IntersectSeriesIDIterators(itr0, itr1 SeriesIDIterator) SeriesIDIterator { // Create series id set, if available. if a := NewSeriesIDSetIterators([]SeriesIDIterator{itr0, itr1}); a != nil { + ss := a[0].SeriesIDSet().And(a[1].SeriesIDSet()) + // `a` holds references to itr0/itr1 so itr0/itr1 should not be closed when `a` is still in use itr0.Close() itr1.Close() - return NewSeriesIDSetIterator(a[0].SeriesIDSet().And(a[1].SeriesIDSet())) + return NewSeriesIDSetIterator(ss) } return &seriesIDIntersectIterator{itrs: [2]SeriesIDIterator{itr0, itr1}} @@ -646,10 +648,11 @@ func UnionSeriesIDIterators(itr0, itr1 SeriesIDIterator) SeriesIDIterator { // Create series id set, if available. if a := NewSeriesIDSetIterators([]SeriesIDIterator{itr0, itr1}); a != nil { - itr0.Close() - itr1.Close() ss := NewSeriesIDSet() ss.Merge(a[0].SeriesIDSet(), a[1].SeriesIDSet()) + // `a` holds references to itr0/itr1 so itr0/itr1 should not be closed when `a` is still in use + itr0.Close() + itr1.Close() return NewSeriesIDSetIterator(ss) } @@ -733,9 +736,11 @@ func DifferenceSeriesIDIterators(itr0, itr1 SeriesIDIterator) SeriesIDIterator { // Create series id set, if available. if a := NewSeriesIDSetIterators([]SeriesIDIterator{itr0, itr1}); a != nil { + ss := a[0].SeriesIDSet().AndNot(a[1].SeriesIDSet()) + // `a` holds references to itr0/itr1 so itr0/itr1 should not be closed when `a` is still in use itr0.Close() itr1.Close() - return NewSeriesIDSetIterator(a[0].SeriesIDSet().AndNot(a[1].SeriesIDSet())) + return NewSeriesIDSetIterator(ss) } return &seriesIDDifferenceIterator{itrs: [2]SeriesIDIterator{itr0, itr1}}