Re: [PATCH] exfat: fix file not locking when writing zeros in exfat_file_mmap()

[Dave Chinner <david@fromorbit.com>]

On Wed, Jan 24, 2024 at 10:05:15AM +0000, Yuezhang.Mo@sony.com wrote:
> From: Matthew Wilcox <willy@infradead.org> 
> Sent: Wednesday, January 24, 2024 1:21 PM
> To: Mo, Yuezhang <Yuezhang.Mo@sony.com>
> Subject: Re: [PATCH] exfat: fix file not locking when writing zeros in exfat_file_mmap()
> > On Wed, Jan 24, 2024 at 05:00:37AM +0000, mailto:Yuezhang.Mo@sony.com wrote:
> > > inode->i_rwsem should be locked when writing file. But the lock
> > > is missing when writing zeros to the file in exfat_file_mmap().
> > 
> > This is actually very weird behaviour in exfat.  This kind of "I must
> > manipulate the on-disc layout" is not generally done in mmap(), it's
> > done in ->page_mkwrite() or even delayed until we actually do writeback.
> > Why does exfat do this?
> 
> In exfat, "valid_size" describes how far into the data stream user data has been
> written and "size" describes the file size.  Return zeros if read "valid_size"~"size".
> 
> For example,
> 
> (1) xfs_io -t -f -c "pwrite -S 0x59 0 1024" $filename
>      - Write 0x59 to 0~1023
>      - both "size" and "valid_size" are 1024
> (2) xfs_io -t -f -c "truncate 4K" $filename
>      - "valid_size" is still 1024
>      - "size" is changed to 4096
>      - 1024~4095 is not zeroed

I think that's the problem right there. File extension via truncate
should really zero the bytes in the page cache in partial pages on
file extension (and likley should do it on-disk as well). See
iomap_truncate_page(), ext4_block_truncate_page(), etc.

Leaving the zeroing until someone actually accesses the data leads
to complexity in the IO path to handle this corner case and getting
that wrong leads directly to data corruption bugs. Just zero the
data in the operation that exposes that data range as zeros to the
user.

-Dave.
-- 
Dave Chinner
david@fromorbit.com