Uploaded image for project: 'IMPALA'
  1. IMPALA
  2. IMPALA-4444

Use-after-free in memory returned from parquet scanner.

    Details

    • Type: Bug
    • Status: Resolved
    • Priority: Blocker
    • Resolution: Fixed
    • Affects Version/s: Impala 2.7.0, Impala 2.8.0
    • Fix Version/s: Impala 2.8.0
    • Component/s: Backend
    • Labels:

      Description

      I can reproduce this by running a local stress test under ASAN:

      ./buildall.sh -asan -noclean -notests -skiptests -ninja -start_impala_cluster
      ./tests/stress/concurrent_select.py --cancel-probability=0.5 --max-queries 10000 --tpch-db=tpch_20_parquet --fail-upon-successive-errors=100
      

      The full log is attached, but the condensed version is:

      ==8612==ERROR: AddressSanitizer: heap-use-after-free on address 0x63100b00480b at pc 0x000000fcc9c5 bp 0x7fb48bb23520 sp 0x7fb48bb22cd0
      READ of size 18 at 0x63100b00480b thread T3933
          #0 0xfcc9c4 in __asan_memcpy /data/jenkins/workspace/verify-impala-toolchain-package-build/label/ec2-package-ubuntu-16-04/toolchain/source/llvm/llvm-3.8.0.src-p1/projects/compiler-rt/lib/asan/asan_interceptors.cc:393
          #1 0x1bf87e5 in impala::BufferedTupleStream::CopyStrings(impala::Tuple const*, std::vector<impala::SlotDescriptor*, std::allocator<impala::SlotDescriptor*> > const&) /tmp/be/src/runtime/buffered-tuple-stream.cc:840:7
          #2 0x1bfb429 in bool impala::BufferedTupleStream::DeepCopyInternal<false>(impala::TupleRow*) /tmp/be/src/runtime/buffered-tuple-stream.cc:815:30
          #3 0x1bf86ac in impala::BufferedTupleStream::DeepCopy(impala::TupleRow*) /tmp/be/src/runtime/buffered-tuple-stream.cc:752:12
          #4 0x7fb56e118464  (<unknown module>)
          #5 0x18921f5 in impala::PartitionedHashJoinNode::GetNext(impala::RuntimeState*, impala::RowBatch*, bool*) /tmp/be/src/exec/partitioned-hash-join-node.cc:538:24
          #6 0x19001c7 in impala::BlockingJoinNode::GetFirstProbeRow(impala::RuntimeState*) /tmp/be/src/exec/blocking-join-node.cc:240:31
          #7 0x188ba03 in impala::PartitionedHashJoinNode::Open(impala::RuntimeState*) /tmp/be/src/exec/partitioned-hash-join-node.cc:187:29
          #8 0x186732b in impala::PartitionedAggregationNode::Open(impala::RuntimeState*) /tmp/be/src/exec/partitioned-aggregation-node.cc:317:29
          #9 0x1c60ece in impala::PlanFragmentExecutor::OpenInternal() /tmp/be/src/runtime/plan-fragment-executor.cc:322:31
          #10 0x1c60a05 in impala::PlanFragmentExecutor::Open() /tmp/be/src/runtime/plan-fragment-executor.cc:295:19
      
      0x63100b00480b is located 11 bytes inside of 65528-byte region [0x63100b004800,0x63100b0147f8)
      freed by thread T3960 here:
          #0 0xfe2620 in __interceptor_free /data/jenkins/workspace/verify-impala-toolchain-package-build/label/ec2-package-ubuntu-16-04/toolchain/source/llvm/llvm-3.8.0.src-p1/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:38
          #1 0x13a1162 in impala::MemPool::FreeAll() /tmp/be/src/runtime/mem-pool.cc:89:5
          #2 0x17cbede in impala::HdfsParquetScanner::AssembleRows(std::vector<impala::ParquetColumnReader*, std::allocator<impala::ParquetColumnReader*> > const&, impala::RowBatch*, bool*) /tmp/be/src/exec/hdfs-parquet-scanner.cc:540:9
          #3 0x17c86f4 in impala::HdfsParquetScanner::GetNextInternal(impala::RowBatch*) /tmp/be/src/exec/hdfs-parquet-scanner.cc:402:19
          #4 0x17c7714 in impala::HdfsParquetScanner::ProcessSplit() /tmp/be/src/exec/hdfs-parquet-scanner.cc:334:31
          #5 0x17592a5 in impala::HdfsScanNode::ProcessSplit(std::vector<impala::FilterContext, std::allocator<impala::FilterContext> > const&, impala::DiskIoMgr::ScanRange*) /tmp/be/src/exec/hdfs-scan-node.cc:527:12
          #6 0x17583df in impala::HdfsScanNode::ScannerThread() /tmp/be/src/exec/hdfs-
      
      previously allocated by thread T3960 here:
          #0 0xfe2928 in __interceptor_malloc /data/jenkins/workspace/verify-impala-toolchain-package-build/label/ec2-package-ubuntu-16-04/toolchain/source/llvm/llvm-3.8.0.src-p1/projects/compiler-rt/lib/asan/asan_malloc_linux.cc:52
          #1 0x139f526 in impala::MemPool::FindChunk(long, bool) /tmp/be/src/runtime/mem-pool.cc:149:45
          #2 0x13042b9 in unsigned char* impala::MemPool::Allocate<true>(long) /tmp/be/src/runtime/mem-pool.h:236:32
          #3 0x180a950 in impala::BaseScalarColumnReader::ReadDataPage() /tmp/be/src/exec/parquet-column-readers.cc:907:11
          #4 0x180e495 in impala::BaseScalarColumnReader::NextPage() /tmp/be/src/exec/parquet-column-readers.cc:989:28
          #5 0x18432d8 in bool impala::ScalarColumnReader<impala::StringValue, true>::ReadValueBatch<false>(impala::MemPool*, int, int, unsigned char*, int*) /tmp/be/src/exec/parquet-column-readers.cc:299:14
          #6 0x17cba79 in impala::HdfsParquetScanner::AssembleRows(std::vector<impala::ParquetColumnReader*, std::allocator<impala::ParquetColumnReader*> > const&, impala::RowBatch*, bool*) /tmp/be/src/exec/hdfs-parquet-scanner.cc:532:30
          #7 0x17c86f4 in impala::HdfsParquetScanner::GetNextInternal(impala::RowBatch*) /tmp/be/src/exec/hdfs-parquet-scanner.cc:402:19
          #8 0x17c7714 in impala::HdfsParquetScanner::ProcessSplit() /tmp/be/src/exec/hdfs-parquet-scanner.cc:334:31
          #9 0x17592a5 in impala::HdfsScanNode::ProcessSplit(std::vector<impala::FilterContext, std::allocator<impala::FilterContext> > const&, impala::DiskIoMgr::ScanRange*) /tmp/be/src/exec/hdfs-scan-node.cc:527:12
          #10 0x17583df in impala::HdfsScanNode::ScannerThread() /tmp/be/src/exec/hdfs-scan-node.cc:418:16
      
      1. impalad.ERROR
        15 kB
        Tim Armstrong

        Activity

        Hide
        tarmstrong Tim Armstrong added a comment -

        The specific query that failed appears to be TPC-H Q10 on scale factor 20 with memory limit of 481mb. It doesn't seem to reproduce every time.

        Show
        tarmstrong Tim Armstrong added a comment - The specific query that failed appears to be TPC-H Q10 on scale factor 20 with memory limit of 481mb. It doesn't seem to reproduce every time.
        Hide
        kwho Michael Ho added a comment -

        The problem stems from the fact that the transfer of MemPool decompressed_data_pool_ in ParquetColumnReaders is deferred until the last scratch tuple batch which exhausts it. In other words, there may be other row batches which have been shipped upstreams still referencing the buffer in decompressed_data_pool_ even though it is still owned by the scanner. In this case, HdfsParquetScanner::AssembleRows() hit an error after creating some row batches from scratch_batch_ and tried to free scratch_batch_ and any mempool attached to it. This inadvertently also frees buffers being referenced upstream by some row batches.

        Show
        kwho Michael Ho added a comment - The problem stems from the fact that the transfer of MemPool decompressed_data_pool_ in ParquetColumnReaders is deferred until the last scratch tuple batch which exhausts it. In other words, there may be other row batches which have been shipped upstreams still referencing the buffer in decompressed_data_pool_ even though it is still owned by the scanner. In this case, HdfsParquetScanner::AssembleRows() hit an error after creating some row batches from scratch_batch_ and tried to free scratch_batch_ and any mempool attached to it. This inadvertently also frees buffers being referenced upstream by some row batches.
        Hide
        kwho Michael Ho added a comment -

        IMPALA-4444: Transfer row group resources to row batch on scan failure

        Previously, if any column reader fails in HdfsParqetScanner::AssembleRows(),
        the memory pools associated with the ScratchTupleBatch will be freed. This
        is problematic as ScratchTupleBatch may contain memory pools which are still
        referenced by row batches shipped upstream. This is possible because memory
        pools used by parquet column readers (e.g. decompressor_pool_) won't be
        transferred to a ScratchTupleBatch until the data page is exhausted. So,
        the memory pools of the previous data page is always attached to the
        ScratchTupleBatch of the current data page. On a scan failure, it's not
        necessarily safe to free the memory pool attached to the current ScratchTupleBatch.

        This patch fixes the problem above by transferring the memory pool and other
        resources associated with a row group to the current row batch in the parquet
        scanner on scan failure so it can eventually be freed by upstream operators as
        the row batch is consumed.

        Change-Id: Id70df470e98dd96284fd176bfbb946e9637ad126
        Reviewed-on: http://gerrit.cloudera.org:8080/5052
        Reviewed-by: Michael Ho <kwho@cloudera.com>
        Tested-by: Internal Jenkins

        Show
        kwho Michael Ho added a comment - IMPALA-4444 : Transfer row group resources to row batch on scan failure Previously, if any column reader fails in HdfsParqetScanner::AssembleRows(), the memory pools associated with the ScratchTupleBatch will be freed. This is problematic as ScratchTupleBatch may contain memory pools which are still referenced by row batches shipped upstream. This is possible because memory pools used by parquet column readers (e.g. decompressor_pool_) won't be transferred to a ScratchTupleBatch until the data page is exhausted. So, the memory pools of the previous data page is always attached to the ScratchTupleBatch of the current data page. On a scan failure, it's not necessarily safe to free the memory pool attached to the current ScratchTupleBatch. This patch fixes the problem above by transferring the memory pool and other resources associated with a row group to the current row batch in the parquet scanner on scan failure so it can eventually be freed by upstream operators as the row batch is consumed. Change-Id: Id70df470e98dd96284fd176bfbb946e9637ad126 Reviewed-on: http://gerrit.cloudera.org:8080/5052 Reviewed-by: Michael Ho <kwho@cloudera.com> Tested-by: Internal Jenkins
        Hide
        dhecht Dan Hecht added a comment -

        Let's leave it a blocker so it shows up as a backport candidate (if needed). If you were leaving it open to remember to add a test, how about resolving this jira and opening a test jira for that?

        Show
        dhecht Dan Hecht added a comment - Let's leave it a blocker so it shows up as a backport candidate (if needed). If you were leaving it open to remember to add a test, how about resolving this jira and opening a test jira for that?

          People

          • Assignee:
            kwho Michael Ho
            Reporter:
            tarmstrong Tim Armstrong
          • Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development