Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-41230

Inconsistent behaviour and integer overflows in usage of qAllocMore

    XMLWordPrintable

Details

    • 880986be2357a1f80827d038d770dc2f80300201

    Description

      • qAllocMore in qbytearray.cpp imposes an arbitrary limit of 1 << 30 to allocation sizes and checks that with a Q_ASSERT_X. That check isn't executed if Qt is compiled without asserts. Any release build will thus happily let the integers overflow and return something random. As we'll probably have to stick to int here the limit should be std::numeric_limits<int>::max() and there should be some graceful error handling.
      • qAllocMore is used in a lot of places, especially in QArrayData::allocate(), which in turn is the base for QVector. In effect that means that you cannot allocate a QVector of more than 1GB total size. In this day and age this limit is easy to hit and the result is not obvious. This little program demonstrates the effect quite nicely. Run it with a debug build of Qt and you get the assert at 28. Run it with a release build of Qt and you get a segfault at 29.
      #include <QVector>
      #include <QDebug>
      
      int main(int, char **)
      {
          QVector<int> x;
          for (int i = 0; i < 40; ++i) {
              qDebug() << i << (1 << i) * sizeof(int);
              x.resize(1 << i);
          }
      }
      
      • The grow() function in qlist.cpp, QString::reallocData() and QFragmentMapData::createFragment() probably have the same problem.
      • In addition to that the values passed into qAllocMore() are never checked for integer overflows before the call when casting them down or doing arithmetics with them.

      If we want to stick to the 1 << 30 limit we should document maximum sizes resulting from that for the various container types and warn that behavior is undefined if those are surpassed. In addition we should check for or prevent integer overflows where we call qAllocMore with computed values as we might artificially lower the limit by that or produce completely arbitrary results. QArrayData::allocate() for example does a qAllocMore on a significantly larger value than it will later use.

      Another way to handle that is just throwing std::bad_alloc if the limit is surpassed. Given that we do that already in some other places it would definitely help consistency.

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            ulherman Ulf Hermann
            ulherman Ulf Hermann
            Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved: