Thrift
  1. Thrift
  2. THRIFT-1248

pointer subtraction in TMemoryBuffer relies on undefined behavior

    Details

    • Type: Bug Bug
    • Status: Resolved
    • Priority: Major Major
    • Resolution: Fixed
    • Affects Version/s: 0.5
    • Fix Version/s: 0.9.3
    • Component/s: C++ - Library
    • Labels:
      None
    • Patch Info:
      Patch Available

      Description

      The ensureCanWrite function in TMemoryBuffer currently "rebases" the buffer pointers by subtracting the original buffer pointer from the pointer newly returned by realloc. While this seems to work fine on my linux setup(I couldn't force a reproducer), pointer subtraction between pointers that we allocated separately is an undefined operation.

      I have run into problems with this on platforms other than linux.

      I am attaching a patch that removes the undefined operation.

      1. 0_7_0_ptrdiff.patch
        1.0 kB
        Anatoly Fayngelerin

        Activity

        Anatoly Fayngelerin created issue -
        Anatoly Fayngelerin made changes -
        Field Original Value New Value
        Attachment 0_7_0_ptrdiff.patch [ 12488206 ]
        Jake Farrell made changes -
        Fix Version/s 0.8 [ 12316293 ]
        Fix Version/s 0.7 [ 12315360 ]
        Jake Farrell made changes -
        Fix Version/s 0.8 [ 12316293 ]
        Jake Farrell made changes -
        Fix Version/s 0.8 [ 12316293 ]
        Jake Farrell made changes -
        Fix Version/s 0.9 [ 12316294 ]
        Fix Version/s 0.8 [ 12316293 ]
        Jake Farrell made changes -
        Fix Version/s 1.0 [ 12318851 ]
        Fix Version/s 0.9 [ 12316294 ]
        Jens Geyer made changes -
        Patch Info Patch Available [ 10042 ]
        Jens Geyer made changes -
        Patch Info Patch Available [ 10042 ]
        Hide
        Jens Geyer added a comment -

        Anatoly Fayngelerin,
        I see the point. Could you please

        • rebase the patch to current trunk
        • have a look whether wBound_ is really calculated correctly after your modifications being applied?

        Thank you!

        Show
        Jens Geyer added a comment - Anatoly Fayngelerin , I see the point. Could you please rebase the patch to current trunk have a look whether wBound_ is really calculated correctly after your modifications being applied? Thank you!
        Hide
        Jens Geyer added a comment -

        Hi Anatoly Fayngelerin,

        any progress on this?

        Show
        Jens Geyer added a comment - Hi Anatoly Fayngelerin , any progress on this?
        James E. King, III made changes -
        Assignee Anatoly Fayngelerin [ fanatoly ] James E. King, III [ jking3 ]
        James E. King, III made changes -
        Fix Version/s 1.0 [ 12318851 ]
        Hide
        ASF GitHub Bot added a comment -

        GitHub user jeking3 opened a pull request:

        https://github.com/apache/thrift/pull/486

        THRIFT-1248 fix TMemoryBuffer pointer arithmetic and add unit test

        You can merge this pull request into a Git repository by running:

        $ git pull https://github.com/jeking3/thrift bugfix/THRIFT-1248-pointer-arithmetic

        Alternatively you can review and apply these changes as the patch at:

        https://github.com/apache/thrift/pull/486.patch

        To close this pull request, make a commit to your master/trunk branch
        with (at least) the following in the commit message:

        This closes #486


        commit 379f6857f087c87a60a933028549534b06a1371b
        Author: Jim King <jim.king@simplivity.com>
        Date: 2015-05-10T12:08:18Z

        THRIFT-1248 fix TMemoryBuffer pointer arithmetic and add unit test


        Show
        ASF GitHub Bot added a comment - GitHub user jeking3 opened a pull request: https://github.com/apache/thrift/pull/486 THRIFT-1248 fix TMemoryBuffer pointer arithmetic and add unit test You can merge this pull request into a Git repository by running: $ git pull https://github.com/jeking3/thrift bugfix/ THRIFT-1248 -pointer-arithmetic Alternatively you can review and apply these changes as the patch at: https://github.com/apache/thrift/pull/486.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #486 commit 379f6857f087c87a60a933028549534b06a1371b Author: Jim King <jim.king@simplivity.com> Date: 2015-05-10T12:08:18Z THRIFT-1248 fix TMemoryBuffer pointer arithmetic and add unit test
        Hide
        ASF GitHub Bot added a comment -

        Github user asfgit closed the pull request at:

        https://github.com/apache/thrift/pull/486

        Show
        ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/thrift/pull/486
        Roger Meier made changes -
        Status Open [ 1 ] Resolved [ 5 ]
        Fix Version/s 0.9.3 [ 12328952 ]
        Resolution Fixed [ 1 ]
        Hide
        Hudson added a comment -

        SUCCESS: Integrated in Thrift #1541 (See https://builds.apache.org/job/Thrift/1541/)
        THRIFT-1248 fix TMemoryBuffer pointer arithmetic and add unit test (roger: rev 6077481139933b927397c7da0088aa4678f9fb3c)

        • lib/cpp/src/thrift/transport/TBufferTransports.cpp
        • lib/cpp/test/TMemoryBufferTest.cpp
        Show
        Hudson added a comment - SUCCESS: Integrated in Thrift #1541 (See https://builds.apache.org/job/Thrift/1541/ ) THRIFT-1248 fix TMemoryBuffer pointer arithmetic and add unit test (roger: rev 6077481139933b927397c7da0088aa4678f9fb3c) lib/cpp/src/thrift/transport/TBufferTransports.cpp lib/cpp/test/TMemoryBufferTest.cpp
        Transition Time In Source Status Execution Times Last Executer Last Execution Date
        Open Open Resolved Resolved
        1380d 23h 6m 1 Roger Meier 10/May/15 13:45

          People

          • Assignee:
            James E. King, III
            Reporter:
            Anatoly Fayngelerin
          • Votes:
            1 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development