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