Re: [PATCH v4] ntfs: convert a named $DATA stream safely when making room in a record

[Wahid Khan]

Hi,

I went through Sashiko's review of v4. The WOF finding is right, and
checking the rest of its report turned up three more problems in the
same loop. I reproduced all four with a forced make-room pass and debug
delays; each fails on v4 and passes on v5, which I am sending as a
reply to this mail:

- WofCompressedData: the stream of a WOF file was converted, but
  ntfs_read_wof_chunk() reads a non-resident stream from the device, so
  the file read stale clusters until writeback (249 of 250 bytes wrong,
  correct after sync). v5 leaves the streams of a WOF file alone.

- An attribute converted in a file with no links left is evicted on the
  last iput() with its dirty folio, so an open file reads its extended
  attribute back as missing. This is older than my patch, the check
  covered only $DATA. v5 converts nothing in such a file.

- A resize of a stream or an EA converted the unnamed data of the file
  under a writer of that file (iomap_iter warnings, recursive
  mrec_lock), because only the resize of the unnamed data itself holds
  its i_rwsem. v5 converts the unnamed data only when it is the
  attribute being resized.

- The conversion of the attribute being resized waited for folio 0 with
  mrec_lock held and hangs against an mmap fault or a splice reader
  (hung task). I wrote in the mail to the v2 thread that this direct
  conversion is safe because the reader waits on i_rwsem first. That
  holds for read(), not for mmap and splice, so it was wrong. v5 uses
  FGP_NOWAIT there too. While such a reader is stuck on the lock the
  resize now fails with -ENOSPC instead of hanging, which I think is the
  better of the two, but tell me if you would rather handle it
  differently.

The six scenarios from the v4 mail still pass, and so does a WOF file
read before and after the conversion. I also ran a mutated-image fuzzer
(about 800 images, KASAN and lockdep) and the resident/non-resident
reader stress test on v5: no crash involves this code. Two things the
fuzzer hit exist without this patch as well, a WARN in iomap_iter() on
the read path of a crafted image and a lockdep report between extent_lock
and the mft runlist lock. I will look at those separately.

Thanks,
Wahid