ext4: validate donor file superblock early in EXT4_IOC_MOVE_EXT

[PATCH] ext4: validate donor file superblock early in EXT4_IOC_MOVE_EXT

Posted by Yun Zhou 2 weeks, 4 days ago

Reject the EXT4_IOC_MOVE_EXT ioctl early if the donor file does not
belong to the same superblock as the original file.  Currently, this
validation is performed inside ext4_move_extents() by
mext_check_validity(), but only after lock_two_nondirectories() has
already acquired the inode locks.  When the donor fd refers to a file
on a different filesystem (e.g., overlayfs), this late validation
creates a circular lock dependency:

  CPU0 (overlayfs write)            CPU1 (ext4 ioctl)
  ----                              ----
  inode_lock(ovl_inode)
                                    mnt_want_write_file(filp)
                                      sb_start_write(ext4_sb)   [sb_writers]
    backing_file_write_iter()
      vfs_iter_write(real_file)
        file_start_write(real_file)
          sb_start_write(ext4_sb)   [blocked by freeze]
                                    lock_two_nondirectories()
                                      inode_lock(ovl_inode)     [blocked]

With a concurrent freeze operation holding sb_writers write side, this
forms a deadlock cycle: CPU0 waits for freeze to complete, freeze waits
for CPU1's sb_writers reader to exit, CPU1 waits for CPU0's inode lock.

Since EXT4_IOC_MOVE_EXT exchanges physical extents between two files,
it fundamentally requires both files to reside on the same ext4
filesystem.  Moving the superblock check before any lock acquisition
is both semantically correct and eliminates the circular dependency
by ensuring that cross-filesystem donor fds are rejected before
sb_writers or inode locks are taken.

Fixes: fcf6b1b729bc ("ext4: refactor ext4_move_extents code base")
Reported-by: syzbot+ad6118a7584b607c67f2@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=ad6118a7584b607c67f2
Signed-off-by: Yun Zhou <yun.zhou@windriver.com>
---
 fs/ext4/ioctl.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index 1d0c3d4bdf47..f7cd419a3218 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -1650,6 +1650,9 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
 		if (!(fd_file(donor)->f_mode & FMODE_WRITE))
 			return -EBADF;
 
+		if (file_inode(filp)->i_sb != file_inode(fd_file(donor))->i_sb)
+			return -EXDEV;
+
 		err = mnt_want_write_file(filp);
 		if (err)
 			return err;
-- 
2.43.0

Re: [PATCH] ext4: validate donor file superblock early in EXT4_IOC_MOVE_EXT

Posted by Theodore Ts'o 2 weeks, 2 days ago

On Mon, 08 Jun 2026 23:25:21 +0800, Yun Zhou wrote:
> Reject the EXT4_IOC_MOVE_EXT ioctl early if the donor file does not
> belong to the same superblock as the original file.  Currently, this
> validation is performed inside ext4_move_extents() by
> mext_check_validity(), but only after lock_two_nondirectories() has
> already acquired the inode locks.  When the donor fd refers to a file
> on a different filesystem (e.g., overlayfs), this late validation
> creates a circular lock dependency:
> 
> [...]

Applied, thanks!

[1/1] ext4: validate donor file superblock early in EXT4_IOC_MOVE_EXT
      commit: c143957520c6c9b5cd72e0de8b52b814f0c576fe

Best regards,
-- 
Theodore Ts'o <tytso@mit.edu>

Re: [PATCH] ext4: validate donor file superblock early in EXT4_IOC_MOVE_EXT

Posted by Jan Kara 2 weeks, 4 days ago

On Mon 08-06-26 23:25:21, Yun Zhou wrote:
> Reject the EXT4_IOC_MOVE_EXT ioctl early if the donor file does not
> belong to the same superblock as the original file.  Currently, this
> validation is performed inside ext4_move_extents() by
> mext_check_validity(), but only after lock_two_nondirectories() has
> already acquired the inode locks.  When the donor fd refers to a file
> on a different filesystem (e.g., overlayfs), this late validation
> creates a circular lock dependency:
> 
>   CPU0 (overlayfs write)            CPU1 (ext4 ioctl)
>   ----                              ----
>   inode_lock(ovl_inode)
>                                     mnt_want_write_file(filp)
>                                       sb_start_write(ext4_sb)   [sb_writers]
>     backing_file_write_iter()
>       vfs_iter_write(real_file)
>         file_start_write(real_file)
>           sb_start_write(ext4_sb)   [blocked by freeze]
>                                     lock_two_nondirectories()
>                                       inode_lock(ovl_inode)     [blocked]
> 
> With a concurrent freeze operation holding sb_writers write side, this
> forms a deadlock cycle: CPU0 waits for freeze to complete, freeze waits
> for CPU1's sb_writers reader to exit, CPU1 waits for CPU0's inode lock.
> 
> Since EXT4_IOC_MOVE_EXT exchanges physical extents between two files,
> it fundamentally requires both files to reside on the same ext4
> filesystem.  Moving the superblock check before any lock acquisition
> is both semantically correct and eliminates the circular dependency
> by ensuring that cross-filesystem donor fds are rejected before
> sb_writers or inode locks are taken.
> 
> Fixes: fcf6b1b729bc ("ext4: refactor ext4_move_extents code base")
> Reported-by: syzbot+ad6118a7584b607c67f2@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=ad6118a7584b607c67f2
> Signed-off-by: Yun Zhou <yun.zhou@windriver.com>

Good catch. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/ioctl.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index 1d0c3d4bdf47..f7cd419a3218 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -1650,6 +1650,9 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
>  		if (!(fd_file(donor)->f_mode & FMODE_WRITE))
>  			return -EBADF;
>  
> +		if (file_inode(filp)->i_sb != file_inode(fd_file(donor))->i_sb)
> +			return -EXDEV;
> +
>  		err = mnt_want_write_file(filp);
>  		if (err)
>  			return err;
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR

Re: [PATCH] ext4: validate donor file superblock early in EXT4_IOC_MOVE_EXT

Posted by Andreas Dilger 2 weeks, 4 days ago

On Jun 8, 2026, at 09:25, Yun Zhou <yun.zhou@windriver.com> wrote:
> 
> Reject the EXT4_IOC_MOVE_EXT ioctl early if the donor file does not
> belong to the same superblock as the original file.  Currently, this
> validation is performed inside ext4_move_extents() by
> mext_check_validity(), but only after lock_two_nondirectories() has
> already acquired the inode locks.  When the donor fd refers to a file
> on a different filesystem (e.g., overlayfs), this late validation
> creates a circular lock dependency:
> 
>  CPU0 (overlayfs write)            CPU1 (ext4 ioctl)
>  ----                              ----
>  inode_lock(ovl_inode)
>                                    mnt_want_write_file(filp)
>                                      sb_start_write(ext4_sb)   [sb_writers]
>    backing_file_write_iter()
>      vfs_iter_write(real_file)
>        file_start_write(real_file)
>          sb_start_write(ext4_sb)   [blocked by freeze]
>                                    lock_two_nondirectories()
>                                      inode_lock(ovl_inode)     [blocked]
> 
> With a concurrent freeze operation holding sb_writers write side, this
> forms a deadlock cycle: CPU0 waits for freeze to complete, freeze waits
> for CPU1's sb_writers reader to exit, CPU1 waits for CPU0's inode lock.
> 
> Since EXT4_IOC_MOVE_EXT exchanges physical extents between two files,
> it fundamentally requires both files to reside on the same ext4
> filesystem.  Moving the superblock check before any lock acquisition
> is both semantically correct and eliminates the circular dependency
> by ensuring that cross-filesystem donor fds are rejected before
> sb_writers or inode locks are taken.
> 
> Fixes: fcf6b1b729bc ("ext4: refactor ext4_move_extents code base")
> Reported-by: syzbot+ad6118a7584b607c67f2@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=ad6118a7584b607c67f2
> Signed-off-by: Yun Zhou <yun.zhou@windriver.com>

Thanks for the patch.  Looks good to me.

Reviewed-by: Andreas Dilger <adilger@dilger.ca <mailto:adilger@dilger.ca>>

> ---
> fs/ext4/ioctl.c | 3 +++
> 1 file changed, 3 insertions(+)
> 
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index 1d0c3d4bdf47..f7cd419a3218 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -1650,6 +1650,9 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd,
>  	if (!(fd_file(donor)->f_mode & FMODE_WRITE))
>  		return -EBADF;
> 
> +	if (file_inode(filp)->i_sb != file_inode(fd_file(donor))->i_sb)
> +		return -EXDEV;
> +
>  	err = mnt_want_write_file(filp);
>  	if (err)
>  		return err;
> -- 
> 2.43.0
> 


Cheers, Andreas

Re: [PATCH] ext4: validate donor file superblock early in EXT4_IOC_MOVE_EXT

Posted by Hillf Danton 2 weeks, 4 days ago

On Mon, 8 Jun 2026 12:20:07 -0600 Andreas Dilger wrote:
> On Jun 8, 2026, at 09:25, Yun Zhou <yun.zhou@windriver.com> wrote:
> > 
> > Reject the EXT4_IOC_MOVE_EXT ioctl early if the donor file does not
> > belong to the same superblock as the original file.  Currently, this
> > validation is performed inside ext4_move_extents() by
> > mext_check_validity(), but only after lock_two_nondirectories() has
> > already acquired the inode locks.  When the donor fd refers to a file
> > on a different filesystem (e.g., overlayfs), this late validation
> > creates a circular lock dependency:
> > 
> >  CPU0 (overlayfs write)            CPU1 (ext4 ioctl)
> >  ----                              ----
> >  inode_lock(ovl_inode)
> >                                    mnt_want_write_file(filp)
> >                                      sb_start_write(ext4_sb)   [sb_writers]
> >    backing_file_write_iter()
> >      vfs_iter_write(real_file)
> >        file_start_write(real_file)
> >          sb_start_write(ext4_sb)   [blocked by freeze]
> >                                    lock_two_nondirectories()
> >                                      inode_lock(ovl_inode)     [blocked]
>
Why does it exist if the locking order on CPU0 is incorrect?

Re: [PATCH] ext4: validate donor file superblock early in EXT4_IOC_MOVE_EXT

Posted by Zhou, Yun 2 weeks, 4 days ago


On 6/9/26 09:15, Hillf Danton wrote:
> CAUTION: This email comes from a non Wind River email account!
> Do not click links or open attachments unless you recognize the sender and know the content is safe.
>
> On Mon, 8 Jun 2026 12:20:07 -0600 Andreas Dilger wrote:
>> On Jun 8, 2026, at 09:25, Yun Zhou <yun.zhou@windriver.com> wrote:
>>> Reject the EXT4_IOC_MOVE_EXT ioctl early if the donor file does not
>>> belong to the same superblock as the original file.  Currently, this
>>> validation is performed inside ext4_move_extents() by
>>> mext_check_validity(), but only after lock_two_nondirectories() has
>>> already acquired the inode locks.  When the donor fd refers to a file
>>> on a different filesystem (e.g., overlayfs), this late validation
>>> creates a circular lock dependency:
>>>
>>>   CPU0 (overlayfs write)            CPU1 (ext4 ioctl)
>>>   ----                              ----
>>>   inode_lock(ovl_inode)
>>>                                     mnt_want_write_file(filp)
>>>                                       sb_start_write(ext4_sb)   [sb_writers]
>>>     backing_file_write_iter()
>>>       vfs_iter_write(real_file)
>>>         file_start_write(real_file)
>>>           sb_start_write(ext4_sb)   [blocked by freeze]
>>>                                     lock_two_nondirectories()
>>>                                       inode_lock(ovl_inode)     [blocked]
> Why does it exist if the locking order on CPU0 is incorrect?
The locking order on CPU0 (overlayfs write path: inode_lock → sb_writers)
is correct and inherent to stacked filesystems.The actual bug is that CPU1's
execution path should never exist — ext4 ioctl should not be locking an
overlayfs inode in the first place.This happens because the donor fd passed
from userspace is not validated before being used in 
lock_two_nondirectories().