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

[Dave Chinner <david@fromorbit.com>]

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.

-Dave.
-- 
Dave Chinner
david@fromorbit.com