Flume
  1. Flume
  2. FLUME-2404

Default maxReadBufferBytes might cause OOM and cause scribe source exit

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: v1.5.0
    • Fix Version/s: v1.6.0
    • Component/s: Sinks+Sources
    • Labels:
      None

      Description

      We're using scribe source, some hacker like infosec guys send some malicious frames to flume with the frame size field set to a very big integer, then the thrift server inside scribe source will exit due to OOM. Then scribe source will keep wait_close state and can not accept any connection.

      1. FLUME-2404.patch
        4 kB
        chenshangan
      2. FLUME-2404-2.patch
        3 kB
        chenshangan

        Issue Links

          Activity

          Hide
          chenshangan added a comment -

          Exception stack is as following:
          16 Jun 2014 15:03:10,526 ERROR [Thread-2] (org.apache.thrift.server.TNonblockingServer$SelectThread.run:247) - run() exiting due to uncaught error
          java.lang.OutOfMemoryError: Java heap space
          at java.nio.HeapByteBuffer.<init>(HeapByteBuffer.java:57)
          at java.nio.ByteBuffer.allocate(ByteBuffer.java:331)
          at org.apache.thrift.server.TNonblockingServer$FrameBuffer.read(TNonblockingServer.java:491)
          at org.apache.thrift.server.TNonblockingServer$SelectThread.handleRead(TNonblockingServer.java:359)
          at org.apache.thrift.server.TNonblockingServer$SelectThread.select(TNonblockingServer.java:304)
          at org.apache.thrift.server.TNonblockingServer$SelectThread.run(TNonblockingServer.java:243)

          Show
          chenshangan added a comment - Exception stack is as following: 16 Jun 2014 15:03:10,526 ERROR [Thread-2] (org.apache.thrift.server.TNonblockingServer$SelectThread.run:247) - run() exiting due to uncaught error java.lang.OutOfMemoryError: Java heap space at java.nio.HeapByteBuffer.<init>(HeapByteBuffer.java:57) at java.nio.ByteBuffer.allocate(ByteBuffer.java:331) at org.apache.thrift.server.TNonblockingServer$FrameBuffer.read(TNonblockingServer.java:491) at org.apache.thrift.server.TNonblockingServer$SelectThread.handleRead(TNonblockingServer.java:359) at org.apache.thrift.server.TNonblockingServer$SelectThread.select(TNonblockingServer.java:304) at org.apache.thrift.server.TNonblockingServer$SelectThread.run(TNonblockingServer.java:243)
          Hide
          chenshangan added a comment - - edited

          FrameBuffer#read

          public boolean read() {
            if (state_ == FrameBufferState.READING_FRAME_SIZE) {
              // try to read the frame size completely
              if (!internalRead()) {
                return false;
              }
              // if the frame size has been read completely, then prepare to read the
              // actual frame.
              if (buffer_.remaining() == 0) {
                // pull out the frame size as an integer.
                int frameSize = buffer_.getInt(0);
                if (frameSize <= 0) {
                  LOGGER.error("Read an invalid frame size of " + frameSize
                      + ". Are you using TFramedTransport on the client side?");
                  return false;
                }
                // if this frame will always be too large for this server, log the
                // error and close the connection.
                if (frameSize > MAX_READ_BUFFER_BYTES) {
                  LOGGER.error("Read a frame size of " + frameSize
                      + ", which is bigger than the maximum allowable buffer size for ALL connections.");
                  return false;
                }
                // if this frame will push us over the memory limit, then return.
                // with luck, more memory will free up the next time around.
                if (readBufferBytesAllocated.get() + frameSize > MAX_READ_BUFFER_BYTES) {
                  return true;
                }
                // increment the amount of memory allocated to read buffers
                readBufferBytesAllocated.addAndGet(frameSize);
                // reallocate the readbuffer as a frame-sized buffer
                buffer_ = ByteBuffer.allocate(frameSize);
                state_ = FrameBufferState.READING_FRAME;
              } else {
                // this skips the check of READING_FRAME state below, since we can't
                // possibly go on to that state if there's data left to be read at
                // this one.
                return true;
              }
            }
          
          

          default MAX_READ_BUFFER_BYTES is Long.MAX_VALUE,obviously it's problematic

          Show
          chenshangan added a comment - - edited FrameBuffer#read public boolean read() { if (state_ == FrameBufferState.READING_FRAME_SIZE) { // try to read the frame size completely if (!internalRead()) { return false ; } // if the frame size has been read completely, then prepare to read the // actual frame. if (buffer_.remaining() == 0) { // pull out the frame size as an integer. int frameSize = buffer_.getInt(0); if (frameSize <= 0) { LOGGER.error( "Read an invalid frame size of " + frameSize + ". Are you using TFramedTransport on the client side?" ); return false ; } // if this frame will always be too large for this server, log the // error and close the connection. if (frameSize > MAX_READ_BUFFER_BYTES) { LOGGER.error( "Read a frame size of " + frameSize + ", which is bigger than the maximum allowable buffer size for ALL connections." ); return false ; } // if this frame will push us over the memory limit, then return . // with luck, more memory will free up the next time around. if (readBufferBytesAllocated.get() + frameSize > MAX_READ_BUFFER_BYTES) { return true ; } // increment the amount of memory allocated to read buffers readBufferBytesAllocated.addAndGet(frameSize); // reallocate the readbuffer as a frame-sized buffer buffer_ = ByteBuffer.allocate(frameSize); state_ = FrameBufferState.READING_FRAME; } else { // this skips the check of READING_FRAME state below, since we can't // possibly go on to that state if there's data left to be read at // this one. return true ; } } default MAX_READ_BUFFER_BYTES is Long.MAX_VALUE,obviously it's problematic
          Hide
          Mike Percy added a comment -

          chenshangan, can you please take a look at FLUME-2405 as well? Seems to be related, and although I am not very familiar with how Scribe works, it seems we should use compatible defaults for both.

          Thanks,
          Mike

          Show
          Mike Percy added a comment - chenshangan , can you please take a look at FLUME-2405 as well? Seems to be related, and although I am not very familiar with how Scribe works, it seems we should use compatible defaults for both. Thanks, Mike
          Hide
          chenshangan added a comment -

          Mike Percy Yes, it looks like that FrameBuffer holds the buffer and provides it to TFramedTransport. So compatible defaults make sense.

          Show
          chenshangan added a comment - Mike Percy Yes, it looks like that FrameBuffer holds the buffer and provides it to TFramedTransport. So compatible defaults make sense.
          Hide
          chenshangan added a comment -

          Mike Percy I'd like to merge the change in FLUME-2405 and provide a new patch. What do you think?

          Show
          chenshangan added a comment - Mike Percy I'd like to merge the change in FLUME-2405 and provide a new patch. What do you think?
          Hide
          Mike Percy added a comment -

          chenshangan Sounds good to me, please go ahead. Thanks!

          Show
          Mike Percy added a comment - chenshangan Sounds good to me, please go ahead. Thanks!
          Hide
          chenshangan added a comment -

          Resolve the issue Flume-2405, and keep the bufferSize conceptually consistent.

          Show
          chenshangan added a comment - Resolve the issue Flume-2405, and keep the bufferSize conceptually consistent.
          Hide
          Mike Percy added a comment -

          +1 lgtm

          Show
          Mike Percy added a comment - +1 lgtm
          Hide
          ASF subversion and git services added a comment -

          Commit f15f20785262ac3cb3e35c2a12e669b7a836d35f in flume's branch refs/heads/trunk from Mike Percy
          [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=f15f207 ]

          FLUME-2404. Make ScribeSource read buffer and max frame size configurable

          Scribe default Thrift service maxReadBufferBytes and frame size varies
          across Thrift versions. In some cases, these values are set to INT_MAX,
          in other cases this is set to 16MB. To avoid OOM in certain cases and
          incompatibilities in other cases, set the default to 16MB and also make
          the parameters configurable.

          (chenshangan and Marimuthu Ponnambalam via Mike Percy)

          Show
          ASF subversion and git services added a comment - Commit f15f20785262ac3cb3e35c2a12e669b7a836d35f in flume's branch refs/heads/trunk from Mike Percy [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=f15f207 ] FLUME-2404 . Make ScribeSource read buffer and max frame size configurable Scribe default Thrift service maxReadBufferBytes and frame size varies across Thrift versions. In some cases, these values are set to INT_MAX, in other cases this is set to 16MB. To avoid OOM in certain cases and incompatibilities in other cases, set the default to 16MB and also make the parameters configurable. (chenshangan and Marimuthu Ponnambalam via Mike Percy)
          Hide
          ASF subversion and git services added a comment -

          Commit 20744dcf6bc14893e89acf1dd44f5208f74e5395 in flume's branch refs/heads/flume-1.6 from Mike Percy
          [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=20744dc ]

          FLUME-2404. Make ScribeSource read buffer and max frame size configurable

          Scribe default Thrift service maxReadBufferBytes and frame size varies
          across Thrift versions. In some cases, these values are set to INT_MAX,
          in other cases this is set to 16MB. To avoid OOM in certain cases and
          incompatibilities in other cases, set the default to 16MB and also make
          the parameters configurable.

          (chenshangan and Marimuthu Ponnambalam via Mike Percy)

          Show
          ASF subversion and git services added a comment - Commit 20744dcf6bc14893e89acf1dd44f5208f74e5395 in flume's branch refs/heads/flume-1.6 from Mike Percy [ https://git-wip-us.apache.org/repos/asf?p=flume.git;h=20744dc ] FLUME-2404 . Make ScribeSource read buffer and max frame size configurable Scribe default Thrift service maxReadBufferBytes and frame size varies across Thrift versions. In some cases, these values are set to INT_MAX, in other cases this is set to 16MB. To avoid OOM in certain cases and incompatibilities in other cases, set the default to 16MB and also make the parameters configurable. (chenshangan and Marimuthu Ponnambalam via Mike Percy)
          Hide
          Mike Percy added a comment -

          Pushed to trunk and flume-1.6. Made a couple minor tweaks on commit.

          Thank you for the patches chenshangan, and also Marimuthu!

          Show
          Mike Percy added a comment - Pushed to trunk and flume-1.6. Made a couple minor tweaks on commit. Thank you for the patches chenshangan, and also Marimuthu!
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in flume-trunk #644 (See https://builds.apache.org/job/flume-trunk/644/)
          FLUME-2404. Make ScribeSource read buffer and max frame size configurable (mpercy: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=f15f20785262ac3cb3e35c2a12e669b7a836d35f)

          • flume-ng-doc/sphinx/FlumeUserGuide.rst
          • flume-ng-sources/flume-scribe-source/src/main/java/org/apache/flume/source/scribe/ScribeSource.java
          Show
          Hudson added a comment - SUCCESS: Integrated in flume-trunk #644 (See https://builds.apache.org/job/flume-trunk/644/ ) FLUME-2404 . Make ScribeSource read buffer and max frame size configurable (mpercy: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=f15f20785262ac3cb3e35c2a12e669b7a836d35f ) flume-ng-doc/sphinx/FlumeUserGuide.rst flume-ng-sources/flume-scribe-source/src/main/java/org/apache/flume/source/scribe/ScribeSource.java
          Hide
          Hudson added a comment -

          SUCCESS: Integrated in Flume-trunk-hbase-98 #4 (See https://builds.apache.org/job/Flume-trunk-hbase-98/4/)
          FLUME-2404. Make ScribeSource read buffer and max frame size configurable (mpercy: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=f15f20785262ac3cb3e35c2a12e669b7a836d35f)

          • flume-ng-sources/flume-scribe-source/src/main/java/org/apache/flume/source/scribe/ScribeSource.java
          • flume-ng-doc/sphinx/FlumeUserGuide.rst
          Show
          Hudson added a comment - SUCCESS: Integrated in Flume-trunk-hbase-98 #4 (See https://builds.apache.org/job/Flume-trunk-hbase-98/4/ ) FLUME-2404 . Make ScribeSource read buffer and max frame size configurable (mpercy: http://git-wip-us.apache.org/repos/asf/flume/repo?p=flume.git&a=commit&h=f15f20785262ac3cb3e35c2a12e669b7a836d35f ) flume-ng-sources/flume-scribe-source/src/main/java/org/apache/flume/source/scribe/ScribeSource.java flume-ng-doc/sphinx/FlumeUserGuide.rst

            People

            • Assignee:
              chenshangan
              Reporter:
              chenshangan
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development