* Re: [LTP] [PATCH] ioctl_ficlone03: fix capabilities on immutable files [not found] <20250326-fix_ioctl_ficlone03_32bit_bcachefs-v1-1-554a0315ebf5@suse.com> @ 2025-03-26 13:47 ` Petr Vorel 2025-03-26 14:12 ` Kent Overstreet 2025-03-27 0:49 ` Dave Chinner 0 siblings, 2 replies; 5+ messages in thread From: Petr Vorel @ 2025-03-26 13:47 UTC (permalink / raw) To: Andrea Cervesato Cc: ltp, Kent Overstreet, linux-bcachefs, linux-fsdevel, Li Wang, Cyril Hrubis 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) > Introduce CAP_LINUX_IMMUTABLE capability to make sure that test won't > fail on other systems as well. > Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> > --- > testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c > index 6a9d270d9fb56f3a263f0aed976f62c4576e6a5f..6716423d9c96d9fc1d433f28e0ae1511b912ae2c 100644 > --- a/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c > +++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c > @@ -122,5 +122,9 @@ static struct tst_test test = { > .bufs = (struct tst_buffers []) { > {&clone_range, .size = sizeof(struct file_clone_range)}, > {}, > - } > + }, > + .caps = (struct tst_cap []) { > + TST_CAP(TST_CAP_REQ, CAP_LINUX_IMMUTABLE), > + {} > + }, Reviewed-by: Petr Vorel <pvorel@suse.cz> LGTM, obviously it is CAP_LINUX_IMMUTABLE related. This looks like fs/bcachefs/fs-ioctl.c static int bch2_inode_flags_set(struct btree_trans *trans, struct bch_inode_info *inode, struct bch_inode_unpacked *bi, void *p) { ... if (((newflags ^ oldflags) & (BCH_INODE_append|BCH_INODE_immutable)) && !capable(CAP_LINUX_IMMUTABLE)) return -EPERM; We don't test on vfat and exfat. If I try to do it fail the same way (bcachefs, fat, exfat and ocfs2 use custom handler for FAT_IOCTL_SET_ATTRIBUTES). I wonder why it does not fail for generic VFS fs/ioctl.c used by Btrfs and XFS: /* * Generic function to check FS_IOC_FSSETXATTR/FS_IOC_SETFLAGS values and reject * any invalid configurations. * * Note: must be called with inode lock held. */ static int fileattr_set_prepare(struct inode *inode, const struct fileattr *old_ma, struct fileattr *fa) { int err; /* * The IMMUTABLE and APPEND_ONLY flags can only be changed by * the relevant capability. */ if ((fa->flags ^ old_ma->flags) & (FS_APPEND_FL | FS_IMMUTABLE_FL) && !capable(CAP_LINUX_IMMUTABLE)) return -EPERM; Kind regards, Petr > }; > --- > base-commit: 753bd13472d4be44eb70ff183b007fe9c5fffa07 > change-id: 20250326-fix_ioctl_ficlone03_32bit_bcachefs-2ec15bd6c0c6 > Best regards, ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [LTP] [PATCH] ioctl_ficlone03: fix capabilities on immutable files 2025-03-26 13:47 ` [LTP] [PATCH] ioctl_ficlone03: fix capabilities on immutable files Petr Vorel @ 2025-03-26 14:12 ` Kent Overstreet 2025-03-26 18:42 ` Petr Vorel 2025-03-27 0:49 ` Dave Chinner 1 sibling, 1 reply; 5+ messages in thread From: Kent Overstreet @ 2025-03-26 14:12 UTC (permalink / raw) To: Petr Vorel Cc: Andrea Cervesato, ltp, linux-bcachefs, linux-fsdevel, Li Wang, Cyril Hrubis 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 ] I'm missing context here, and the original thread doesn't seem to be on lore? > > > 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) > > > Introduce CAP_LINUX_IMMUTABLE capability to make sure that test won't > > fail on other systems as well. > > > Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> > > --- > > testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c > > index 6a9d270d9fb56f3a263f0aed976f62c4576e6a5f..6716423d9c96d9fc1d433f28e0ae1511b912ae2c 100644 > > --- a/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c > > +++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c > > @@ -122,5 +122,9 @@ static struct tst_test test = { > > .bufs = (struct tst_buffers []) { > > {&clone_range, .size = sizeof(struct file_clone_range)}, > > {}, > > - } > > + }, > > + .caps = (struct tst_cap []) { > > + TST_CAP(TST_CAP_REQ, CAP_LINUX_IMMUTABLE), > > + {} > > + }, > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > LGTM, obviously it is CAP_LINUX_IMMUTABLE related. > > This looks like fs/bcachefs/fs-ioctl.c > > static int bch2_inode_flags_set(struct btree_trans *trans, > struct bch_inode_info *inode, > struct bch_inode_unpacked *bi, > void *p) > { > ... > if (((newflags ^ oldflags) & (BCH_INODE_append|BCH_INODE_immutable)) && > !capable(CAP_LINUX_IMMUTABLE)) > return -EPERM; > > > We don't test on vfat and exfat. If I try to do it fail the same way (bcachefs, > fat, exfat and ocfs2 use custom handler for FAT_IOCTL_SET_ATTRIBUTES). > > I wonder why it does not fail for generic VFS fs/ioctl.c used by Btrfs and XFS: > > /* > * Generic function to check FS_IOC_FSSETXATTR/FS_IOC_SETFLAGS values and reject > * any invalid configurations. > * > * Note: must be called with inode lock held. > */ > static int fileattr_set_prepare(struct inode *inode, > const struct fileattr *old_ma, > struct fileattr *fa) > { > int err; > > /* > * The IMMUTABLE and APPEND_ONLY flags can only be changed by > * the relevant capability. > */ > if ((fa->flags ^ old_ma->flags) & (FS_APPEND_FL | FS_IMMUTABLE_FL) && > !capable(CAP_LINUX_IMMUTABLE)) > return -EPERM; > > > Kind regards, > Petr > > > }; > > > --- > > base-commit: 753bd13472d4be44eb70ff183b007fe9c5fffa07 > > change-id: 20250326-fix_ioctl_ficlone03_32bit_bcachefs-2ec15bd6c0c6 > > > Best regards, ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [LTP] [PATCH] ioctl_ficlone03: fix capabilities on immutable files 2025-03-26 14:12 ` Kent Overstreet @ 2025-03-26 18:42 ` Petr Vorel 0 siblings, 0 replies; 5+ messages in thread From: Petr Vorel @ 2025-03-26 18:42 UTC (permalink / raw) To: Kent Overstreet Cc: Andrea Cervesato, ltp, linux-bcachefs, linux-fsdevel, Li Wang, Cyril Hrubis Hi Kent, > 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 ] > I'm missing context here, and the original thread doesn't seem to be on > lore? I'm sorry, to put more info: this is an attempt to fix of LTP test ioctl_ficlone03.c [1], which is failing on 32 bit compatibility mode: # ./ioctl_ficlone03 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) Original thread of this is on LTP ML [2]. Kind regards, Petr [1] https://github.com/linux-test-project/ltp/tree/master/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c [2] https://lore.kernel.org/ltp/20250326-fix_ioctl_ficlone03_32bit_bcachefs-v1-1-554a0315ebf5@suse.com/ > > > 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) > > > Introduce CAP_LINUX_IMMUTABLE capability to make sure that test won't > > > fail on other systems as well. > > > Signed-off-by: Andrea Cervesato <andrea.cervesato@suse.com> > > > --- > > > testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c | 6 +++++- > > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > diff --git a/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c > > > index 6a9d270d9fb56f3a263f0aed976f62c4576e6a5f..6716423d9c96d9fc1d433f28e0ae1511b912ae2c 100644 > > > --- a/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c > > > +++ b/testcases/kernel/syscalls/ioctl/ioctl_ficlone03.c > > > @@ -122,5 +122,9 @@ static struct tst_test test = { > > > .bufs = (struct tst_buffers []) { > > > {&clone_range, .size = sizeof(struct file_clone_range)}, > > > {}, > > > - } > > > + }, > > > + .caps = (struct tst_cap []) { > > > + TST_CAP(TST_CAP_REQ, CAP_LINUX_IMMUTABLE), > > > + {} > > > + }, > > Reviewed-by: Petr Vorel <pvorel@suse.cz> > > LGTM, obviously it is CAP_LINUX_IMMUTABLE related. > > This looks like fs/bcachefs/fs-ioctl.c > > static int bch2_inode_flags_set(struct btree_trans *trans, > > struct bch_inode_info *inode, > > struct bch_inode_unpacked *bi, > > void *p) > > { > > ... > > if (((newflags ^ oldflags) & (BCH_INODE_append|BCH_INODE_immutable)) && > > !capable(CAP_LINUX_IMMUTABLE)) > > return -EPERM; > > We don't test on vfat and exfat. If I try to do it fail the same way (bcachefs, > > fat, exfat and ocfs2 use custom handler for FAT_IOCTL_SET_ATTRIBUTES). > > I wonder why it does not fail for generic VFS fs/ioctl.c used by Btrfs and XFS: > > /* > > * Generic function to check FS_IOC_FSSETXATTR/FS_IOC_SETFLAGS values and reject > > * any invalid configurations. > > * > > * Note: must be called with inode lock held. > > */ > > static int fileattr_set_prepare(struct inode *inode, > > const struct fileattr *old_ma, > > struct fileattr *fa) > > { > > int err; > > /* > > * The IMMUTABLE and APPEND_ONLY flags can only be changed by > > * the relevant capability. > > */ > > if ((fa->flags ^ old_ma->flags) & (FS_APPEND_FL | FS_IMMUTABLE_FL) && > > !capable(CAP_LINUX_IMMUTABLE)) > > return -EPERM; > > Kind regards, > > Petr > > > }; > > > --- > > > base-commit: 753bd13472d4be44eb70ff183b007fe9c5fffa07 > > > change-id: 20250326-fix_ioctl_ficlone03_32bit_bcachefs-2ec15bd6c0c6 > > > Best regards, ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [LTP] [PATCH] ioctl_ficlone03: fix capabilities on immutable files 2025-03-26 13:47 ` [LTP] [PATCH] ioctl_ficlone03: fix capabilities on immutable files Petr Vorel 2025-03-26 14:12 ` Kent Overstreet @ 2025-03-27 0:49 ` Dave Chinner 2025-03-28 16:26 ` Kent Overstreet 1 sibling, 1 reply; 5+ messages in thread From: Dave Chinner @ 2025-03-27 0:49 UTC (permalink / raw) To: Petr Vorel Cc: Andrea Cervesato, ltp, Kent Overstreet, linux-bcachefs, linux-fsdevel, Li Wang, Cyril Hrubis 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [LTP] [PATCH] ioctl_ficlone03: fix capabilities on immutable files 2025-03-27 0:49 ` Dave Chinner @ 2025-03-28 16:26 ` Kent Overstreet 0 siblings, 0 replies; 5+ messages in thread From: Kent Overstreet @ 2025-03-28 16:26 UTC (permalink / raw) To: Dave Chinner Cc: Petr Vorel, Andrea Cervesato, ltp, linux-bcachefs, linux-fsdevel, Li Wang, Cyril Hrubis 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 ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-03-28 16:26 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
[not found] <20250326-fix_ioctl_ficlone03_32bit_bcachefs-v1-1-554a0315ebf5@suse.com>
2025-03-26 13:47 ` [LTP] [PATCH] ioctl_ficlone03: fix capabilities on immutable files Petr Vorel
2025-03-26 14:12 ` Kent Overstreet
2025-03-26 18:42 ` Petr Vorel
2025-03-27 0:49 ` Dave Chinner
2025-03-28 16:26 ` Kent Overstreet
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox