Re: [PATCH v4 9/9] ext4: protect WRITE_ZEROES written extents with orphan list

"Theodore Tso" <[email protected]>
Newsgroups org.kernel.vger.linux-ext4,dev.linux.lists.sashiko-reviews
Message-ID <[email protected]>
On Tue, Jul 14, 2026 at 08:42:03AM -0500, [email protected] wrote:
> > @@ -4701,6 +4729,11 @@ static int ext4_alloc_file_blocks(struct file *file, loff_t offset, loff_t len,
> >  		pagecache_isize_extended(inode, old_size, epos);
> >  
> >  	return ret ? ret : ret2;
> > +
> > +out:
> > +	if (orphan && inode->i_nlink)
> > +		ext4_orphan_del(NULL, inode);
> 
> [Severity: Critical]
> Does passing a NULL handle to ext4_orphan_del() here corrupt the on-disk
> orphan list?
> 
> If ext4_convert_unwritten_extents() fails on the first iteration, epos is 0.
> The code skips starting a new journal handle, jumps to out: and calls
> ext4_orphan_del(NULL, inode).
> 
> A NULL handle removes the inode from the in-memory list but leaves it
> permanently linked in the on-disk list.  If this inode is later unlinked and
> freed, could this sever the rest of the on-disk orphan list and lead to severe
> filesystem corruption during recovery if the inode is reallocated?

This looks like a valid problem; it's possible to get to this point in
the code:

        if (!epos || !new_size)
                goto out;

where we've already added the inode to the orphan list, and we will
then leave the inode on the orphan list.

I think we need to fix this by checking to see if orphan == true, and
if so, we need to start a handle, and then call ext4_orphan_del() with
that handle, and stop it afterwards.

Do you agree?

If so, are you OK if I just drop this last patch in the series, and
you can resubmit with the fix?

Thaks,

						- Ted
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.