Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

QUsbEndpoint::readData may throw std::bad_alloc exception. #81

Open
jerry-yuan opened this issue Jun 21, 2021 · 0 comments
Open

QUsbEndpoint::readData may throw std::bad_alloc exception. #81

jerry-yuan opened this issue Jun 21, 2021 · 0 comments
Assignees
Labels

Comments

@jerry-yuan
Copy link

Describe the bug
As following, QUsbEndpoint::readData(char*,quint64) will be invoked by QIODevice::read(quint64):

QByteArray QIODevice::read(qint64 maxSize)
{
    Q_D(QIODevice);
    QByteArray result;

#if defined QIODEVICE_DEBUG
    printf("%p QIODevice::read(%lld), d->pos = %lld, d->buffer.size() = %lld\n",
           this, maxSize, d->pos, d->buffer.size());
#endif

    // Try to prevent the data from being copied, if we have a chunk
    // with the same size in the read buffer.
    if (maxSize == d->buffer.nextDataBlockSize() && !d->transactionStarted
        && (d->openMode & (QIODevice::ReadOnly | QIODevice::Text)) == QIODevice::ReadOnly) {
        result = d->buffer.read();
        if (!d->isSequential())
            d->pos += maxSize;
        if (d->buffer.isEmpty())
             // this line will invoke with a null pointer as parameter
            readData(nullptr, 0);
        return result;
    }

    CHECK_MAXLEN(read, result);
    CHECK_MAXBYTEARRAYSIZE(read);

    result.resize(int(maxSize));
    qint64 readBytes = read(result.data(), result.size());
...

And in line 18 of QIODevice::read(quint64), the QUsbEndpoint::readData(char*,quint64) would be invoked with nullptr as the first parameter when the d->buffer is empty. While in QUsbEndpoint::readData(char*,quint64) there is an assert to make sure the data pointer is not null:

qint64 QUsbEndpoint::readData(char *data, qint64 maxSize)
{
    if (this->openMode() != ReadOnly)
        return -1;

    Q_D(QUsbEndpoint);
    DbgPrintFuncName();
   // this may throw a bad_alloc exception and kill the process.
    Q_CHECK_PTR(data);

    if (maxSize <= 0)
        return 0;

    QMutexLocker locker(&d->m_buf_mutex);
    qint64 read_size = d->m_buf.size();
...

PLatform:

  • OS: Windows
  • Version: 10
  • Qt: 5.12.9

To Reproduce
Steps to reproduce the behavior:

  1. open an input endpoint
  2. let the usb device send n bytes
  3. read the n bytes by invoke QUsbEndpoint::read(quint64) twice:
endpoint->read(m);        // make sure m<n
endpoint->read(n-m);    // this line will crash the process
  1. The process will crash due to the assert.

tips: If you read the remain bytes in buffer by readAll instead, you will not receive this exception.

Expected behavior
A clear and concise description of what you expected to happen.
I'd like to read the entire data in several parts.

Additional context
Add any other context about the problem here.
I tried to search the points that invoke readData with nullptr as first parameter and find there are two points. These two points invoke the readData with 0 as second parameter. For this reason, I try to move Q_CHECK_PTR(data) to after the checking of maxSize:

    // Q_CHECK_PTR(data);  // this line is moved.
    if (maxSize <= 0)
        return 0;
    Q_CHECK_PTR(data);   

It seems work but I am not sure if this will lead to any other issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants