[PATCH 01/15] ubifs: Set page uptodate in the correct place

[Zhihao Cheng]

在 2024/1/22 22:40, Matthew Wilcox 写道:
> On Mon, Jan 22, 2024 at 03:22:45PM +0800, Zhihao Cheng wrote:
>> 在 2024/1/21 7:08, Matthew Wilcox (Oracle) 写道:
>>> Page cache reads are lockless, so setting the freshly allocated page
>>> uptodate before we've overwritten it with the data it's supposed to have
>>> in it will allow a simultaneous reader to see old data.  Move the call
>>> to SetPageUptodate into ubifs_write_end(), which is after we copied the
>>> new data into the page.
>>
>> This solution looks good to me, and I think 'SetPageUptodate' should be
>> removed from write_begin_slow(slow path) too.
> 
> I didn't bother because we have just read into the page so it is
> uptodate.  A racing read will see the data from before the write, but
> that's an acceptable ordering of events.
> .
> 

I can't find where the page is read and set uptodate. I think the 
uninitialized data can be found in following path:

       writer               reader
ubifs_write_begin
  page1 = grab_cache_page_write_begin
  err = allocate_budget // ENOSPC
  unlock_page(page1)
  put_page(page1)
  write_begin_slow
   page2 = grab_cache_page_write_begin
   SetPageChecked(page2)
   SetPageUptodate(page2)
                 generic_file_read_iter
                  filemap_read
                   filemap_get_pages
                    filemap_get_read_batch
                    if (!folio_test_uptodate) // page2 is uptodate
                   copy_folio_to_iter // read uninitialized page content!
copy_page_from_iter_atomic // copy data to cover uninitialized page content