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

QSaveFile truncates file due to incorrect commit/flush handling

    XMLWordPrintable

    Details

    • Type: Bug
    • Status: Reported
    • Priority: P2: Important
    • Resolution: Unresolved
    • Affects Version/s: 5.11.2, 5.13.0 Beta 1
    • Fix Version/s: None
    • Component/s: Core: I/O
    • Labels:
      None
    • Environment:
      Tested Linux 4.20.15, glibc 2.27, but should not be platform-specific
    • Platform/s:
      All

      Description

      QSaveFile saves any write errors in its reimplementation of QSaveFile::writeData() to d->writeError so that QSaveFile::commit() can fail before the atomic file replacement.

      However, there is no equivalent handling for QSaveFile::flush() ( = QFileDevice::flush()). Any errors from flush() are not reflected in d->writeError and commit() will proceed with the file replacement as usual.

      This is a problem even if flush() is not called by the user, as QSaveFile::commit() calls QFileDevice::close() which calls QFileDevice::flush().

      The above most severely breaks QSaveFile on filesystems that allow creation of files when the file system is full, e.g. common Linux filesystems ext4 and tmpfs, as a very simple testcase (write short file while FS full) causes the target file to be replaced with a zero-sized file:

       

      QSaveFile a("file");
      a.open(QSaveFile::WriteOnly);
      a.write("data"); // succeeds - "data" is buffered
      a.commit(); // truncates file if ext4/tmpfs filesystem was full
      

      Other filesystems are affected as well if there is enough space to create the file but not enough space to write all the data out. I did not check which group Win/NTFS falls on. The issue is in QSaveFile platform-agnostic code.

      Any other write error encountered by flush() is also ignored by commit().

       

      Attached is a full testcase application, the issue can be reproduced with it on Linux as follows:

      $ mkdir /tmp/smallfs
      $ sudo mount -ttmpfs -o size=1M none /tmp/smallfs
      $ ./savefiletest /tmp/smallfs/testfile data1
      open: true
      write: 5
      commit: true
      $ cat /tmp/smallfs/testfile
      data1$
      $ dd if=/dev/zero of=/tmp/smallfs/foo
      dd: writing to '/tmp/smallfs/foo': No space left on device
      2041+0 records in
      2040+0 records out
      $ ./savefiletest /tmp/smallfs/testfile data2
      open: true
      write: 5
      commit: true
      $ cat /tmp/smallfs/testfile
      $ ls -l /tmp/smallfs/testfile
      -rw-rw-r--. 1 anssiha anssiha 0 10. 4. 13:00 /tmp/smallfs/testfile
      

      File was truncated and QSaveFile reported no errors.

      The issue is present in dev branch as well.

      Handling full partition is explicitly mentioned in QSaveFile documentation as supported case.

        Attachments

        1. savefiletest.cc
          0.3 kB
        2. savefiletest.pro
          0.1 kB
        3. qtbug-75077-5.11.2.patch
          5 kB
        4. qtbug-75077-5.11.2-v2.patch
          4 kB

          Issue Links

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

            Activity

              People

              • Assignee:
                thiago Thiago Macieira
                Reporter:
                anssi Anssi Hannula
              • Votes:
                0 Vote for this issue
                Watchers:
                4 Start watching this issue

                Dates

                • Created:
                  Updated:

                  Gerrit Reviews

                  There are no open Gerrit changes