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@xxxxxxxx> > > > > > 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
