[LTP] [PATCH] ioctl_ficlone03: fix capabilities on immutable files

[Kent Overstreet]

On Thu, Mar 27, 2025 at 11:49:19AM +1100, Dave Chinner wrote:
> On Wed, Mar 26, 2025 at 02:47:49PM +0100, Petr Vorel wrote:
> > Hi all,
> > 
> > [ Cc Kent and other filesystem folks to be sure we don't hide a bug ]
> > 
> > > From: Andrea Cervesato <andrea.cervesato@suse.com>
> > 
> > > Make sure that capabilities requirements are satisfied when accessing
> > > immutable files. On OpenSUSE Tumbleweed 32bit bcachefs fails with the
> > > following error due to missing capabilities:
> > 
> > > tst_test.c:1833: TINFO: === Testing on bcachefs ===
> > > ..
> > > ioctl_ficlone03.c:74: TBROK: ioctl .. failed: ENOTTY (25)
> > > ioctl_ficlone03.c:89: TWARN: ioctl ..  failed: ENOTTY (25)
> > > ioctl_ficlone03.c:91: TWARN: ioctl ..  failed: ENOTTY (25)
> > > ioctl_ficlone03.c:98: TWARN: close(-1) failed: EBADF (9)
> 
> None of these are -EPERM, so how is a missing capability that
> results in -EPERM being returned cause -ENOTTY or -EBADF failures?
> 
> ohhhhh. ENOTTY is a result of a kernel side compat ioctl handling bug
> w/ bcachefs.
> 
> bcachefs doesn't implement ->fileattr_set().
> 
> sys_compat_ioctl() does:
> 
>         case FS_IOC32_GETFLAGS:
>         case FS_IOC32_SETFLAGS:
>                 cmd = (cmd == FS_IOC32_GETFLAGS) ?
>                         FS_IOC_GETFLAGS : FS_IOC_SETFLAGS;
> 
> and then calls do_vfs_ioctl().
> 
> This then ends up in vfs_fileattr_set(), which does:
> 
> 	if (!inode->i_op->fileattr_set)
>                 return -ENOIOCTLCMD;
> 
> which means sys_compat_ioctl() then falls back to calling
> ->compat_ioctl().
> 
> However, cmd has been overwritten to FS_IOC_SETFLAGS and
> bch2_compat_fs_ioctl() looks for FS_IOC32_SETFLAGS, so it returns
> -ENOIOCTLCMD as it doesn't handle the ioctl command passed in.
> 
> sys_compat_ioctl() then converts the -ENOIOCTLCMD to -ENOTTY, and
> there's the error being reported.
> 
> OK, this is a bcachefs compat ioctl handling bug, triggered by not
> implementing ->fileattr_set and implementing FS_IOC_SETFLAGS
> directly itself via ->unlocked_ioctl.
> 
> Yeah, fixes to bcachefs needed here, not LTP.
> 
> > I wonder why it does not fail for generic VFS fs/ioctl.c used by Btrfs and XFS:
> 
> Because they implement ->fileattr_set() and so all the compat ioctl
> FS_IOC32_SETFLAGS handling works as expected.

thanks for the analysis, I'll try to get this fixed soon