Huaisi Xu has posted comments on this change. Change subject: IMPALA-3494: Fix Thrift TMemoryBuffer overflow ......................................................................
Patch Set 1: (1 comment) http://gerrit.cloudera.org:8080/#/c/3149/1/source/thrift/thrift-0.9.0-patches/0005-IMPALA-3494-fix-Thrift-TMemoryBuffer-overflow.patch File source/thrift/thrift-0.9.0-patches/0005-IMPALA-3494-fix-Thrift-TMemoryBuffer-overflow.patch: Line 10: TMemoryBuffer reached 2 GB, unable to expand > Let me explain what I mean with this comment. The error message indicates t thanks for explaining. the code is like void TMemoryBuffer::ensureCanWrite(uint32_t len) { // Check available space uint32_t avail = available_write(); if (len <= avail) { return; } if (!owner_) { throw TTransportException("Insufficient space in external MemoryBuffer"); } // Grow the buffer as necessary. uint32_t new_size = bufferSize_; while (len > avail) { new_size = new_size > 0 ? new_size * 2 : 1; avail = available_write() + (new_size - bufferSize_); } // Allocate into a new pointer so we don't bork ours if it fails. void* new_buffer = std::realloc(buffer_, new_size); if (new_buffer == NULL) { throw std::bad_alloc(); } bufferSize_ = new_size; ptrdiff_t offset = (uint8_t*)new_buffer - buffer_; buffer_ += offset; rBase_ += offset; rBound_ += offset; wBase_ += offset; wBound_ = buffer_ + bufferSize_; } so when new_size is expanded thirft will request void* new_buffer = std::realloc(buffer_, new_size); immediately. I think if new_size overflows this avail = available_write() + (new_size - bufferSize_); will also overflow. (new_size probably less than bufferSize_) -- To view, visit http://gerrit.cloudera.org:8080/3149 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: I91086ed9d0b4f95b5532de67fe35b46b748ad16c Gerrit-PatchSet: 1 Gerrit-Project: Toolchain Gerrit-Branch: master Gerrit-Owner: Huaisi Xu <[email protected]> Gerrit-Reviewer: Dimitris Tsirogiannis <[email protected]> Gerrit-Reviewer: Huaisi Xu <[email protected]> Gerrit-HasComments: Yes
