Re: [PATCH] shmem: do not extend i_size when no data was copied

[Jan Kara]

On Mon 26-10-26 18:18:39, Kiryl Shutsemau wrote:
> On Mon, Oct 26, 2026 at 04:48:58PM +0100, Jan Kara wrote:
> > On Mon 26-10-26 15:34:08, Kiryl Shutsemau wrote:
> > > On Sun, Oct 25, 2026 at 04:46:19PM +0000, Hungyu Lin wrote:
> > > > shmem_write_end() advances i_size to pos + copied even when copied is
> > > > zero. A buffered write past EOF can therefore extend a tmpfs file
> > > > without writing any data.
> > > 
> > > + willy, jack, fsdevel.
> > > 
> > > This doesn't seem to be tmpfs-specific, right? I see the same with ext4.
> > > 
> > > simple_write_end() and generic_write_end() check pos + copied the same
> > > way, so filesystems that use them through generic_perform_write() are
> > > affected too.
> > > 
> > > It makes me think maybe generic_perform_write() is the right place to
> > > fix it. Hm?
> > 
> > Well, that would be nice. But how'd you like to do that? i_size update is
> > the responsibility of ->write_end and we have to call ->write_end if copied
> > == 0 so that is undoes what ->write_begin has prepared.
> 
> Hm.
> 
> A zero-length copy only moves i_size if the write starts beyond EOF.
> So we can bring the prefault back just for that case, before
> ->write_begin():
> 
>         if (unlikely(pos > i_size_read(mapping->host)) &&
>             fault_in_iov_iter_readable(i, bytes) == bytes) {
>                 status = -EFAULT;
>                 break;
>         }
> 
> Overwrites and appends keep skipping the prefault. It is racy against
> a concurrent munmap(), as it was before 665575cff098, but that is
> userspace racing with itself.
> 
> The per-filesystem alternative is a 'copied &&' in every ->write_end.
> 
> And generic_write_end() cannot take it as is: generic_cont_expand_simple()
> relies on a zero-length ->write_end() extending i_size. So we need some
> replacement.

Yeah, I actually like this. Good idea.

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR