* [GIT PULL] fs idmapped updates for v6.2
@ 2022-12-12 13:19 Christian Brauner
2022-12-13 3:40 ` Linus Torvalds
2022-12-13 3:49 ` [GIT PULL] fs idmapped updates for v6.2 pr-tracker-bot
0 siblings, 2 replies; 6+ messages in thread
From: Christian Brauner @ 2022-12-12 13:19 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Christian Brauner, linux-fsdevel, linux-kernel
Hey Linus,
/* Summary */
Last cycle we've already made the interaction with idmapped mounts more robust
and type safe by introducing the vfs{g,u}id_t type. This cycle we concluded the
conversion and removed the legacy helpers.
Currently we still pass around the plain namespace that was attached to a
mount. This is in general pretty convenient but it makes it easy to conflate
namespaces that are relevant on the filesystem- with namespaces that are
relevent on the mount level. Especially for filesystem developers without
detailed knowledge in this area this can be a potential source for bugs.
Instead of passing the plain namespace we introduce a dedicated type struct
mnt_idmap and replace the pointer with a pointer to a struct mnt_idmap. There
are no semantic or size changes for the mount struct caused by this.
We then start converting all places aware of idmapped mounts to rely on struct
mnt_idmap. Once the conversion is done all helpers down to the really low-level
make_vfs{g,u}id() and from_vfs{g,u}id() will take a struct mnt_idmap argument
instead of two namespace arguments. This way it becomes impossible to conflate
the two removing and thus eliminating the possibility of any bugs. Fwiw, I
fixed some issues in that area a while ago in ntfs3 and ksmbd in the past.
Afterwards only low-level code can ultimately use the associated namespace for
any permission checks. Even most of the vfs can be completely obivious about
this ultimately and filesystems will never interact with it in any form in the
future.
A struct mnt_idmap currently encompasses a simple refcount and pointer to the
relevant namespace the mount is idmapped to. If a mount isn't idmapped then it
will point to a static nop_mnt_idmap and if it doesn't that it is idmapped. As
usual there are no allocations or anything happening for non-idmapped mounts.
Everthing is carefully written to be a nop for non-idmapped mounts as has
always been the case.
If an idmapped mount is created a struct mnt_idmap is allocated and a reference
taken on the relevant namespace. Each mount that gets idmapped or inherits the
idmap simply bumps the reference count on struct mnt_idmap. Just a reminder
that we only allow a mount to change it's idmapping a single time and only if
it hasn't already been attached to the filesystems and has no active writers.
The actual changes are fairly straightforward but this will have huge benefits
for maintenance and security in the long run even if it causes some churn.
Note that this also makes it possible to extend struct mount_idmap in the
future. For example, it would be possible to place the namespace pointer in an
anonymous union together with an idmapping struct. This would allow us to
expose an api to userspace that would let it specify idmappings directly
instead of having to go through the detour of setting up namespaces at all.
/* Testing */
clang: Ubuntu clang version 15.0.2-1
gcc: gcc (Ubuntu 12.2.0-3ubuntu1) 12.2.0
All patches are based on v6.1-rc1 and have been sitting in linux-next. No build
failures or warnings were observed. All old and new tests in fstests,
selftests, and LTP pass without regressions.
/* Conflicts */
At the time of creating this PR no merge conflicts were reported from
linux-next and no merge conflicts showed up doing a test-merge with current
mainline.
The following changes since commit 9abf2313adc1ca1b6180c508c25f22f9395cc780:
Linux 6.1-rc1 (2022-10-16 15:36:24 -0700)
are available in the Git repository at:
ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.idmapped.mnt_idmap.v6.2
for you to fetch changes up to 5a6f52d20ce3cd6d30103a27f18edff337da191b:
acl: conver higher-level helpers to rely on mnt_idmap (2022-10-31 17:48:12 +0100)
Please note the tag contains all other branches for this cycle merged in.
They're on the list at:
(1) vfsuid updates for v6.2
https://lore.kernel.org/lkml/20221212123348.169903-1-brauner@kernel.org
(2) setgid inheritance updates for v6.2
https://lore.kernel.org/lkml/20221212112053.99208-1-brauner@kernel.org
(3) acl updates for v6.2
https://lore.kernel.org/lkml/20221212111919.98855-1-brauner@kernel.org
The
256c8aed2b42 ("fs: introduce dedicated idmap type for mounts")
introduces the infrastructure and
5a6f52d20ce3 ("acl: conver higher-level helpers to rely on mnt_idmap")
builds upon the acl work to convert over a first set of high-level helpers that
don't cause a lot of churn because they have been newly introduced this cycle.
Since it touches the core idmapping header it would cause really a lot of
conflicts with the vfsuid patchset. While I'm aware that you usually don't mind
it would've made things a lot more annoying. So the vfsuid changes are merged
in as well which bring in the setgid changes.
I really prefer splitting pull requests into topcis rather than large patch
dumps but then I run into limits when there's dependencies between the
different topics involved. So far I haven't found out an optimal way of doing
this and I hope this way here is still acceptable. If you have a better way or
would provide me a different tag or branch I'm happy to do so.
Thanks!
Christian
----------------------------------------------------------------
fs.idmapped.mnt_idmap.v6.2
----------------------------------------------------------------
Amir Goldstein (2):
ovl: remove privs in ovl_copyfile()
ovl: remove privs in ovl_fallocate()
Christian Brauner (51):
attr: add in_group_or_capable()
fs: move should_remove_suid()
attr: add setattr_should_drop_sgid()
attr: use consistent sgid stripping checks
Merge branch 'fs.ovl.setgid' into for-next
orangefs: rework posix acl handling when creating new filesystem objects
fs: pass dentry to set acl method
fs: rename current get acl method
fs: add new get acl method
cifs: implement get acl method
cifs: implement set acl method
9p: implement get acl method
9p: implement set acl method
security: add get, remove and set acl hook
selinux: implement get, set and remove acl hook
smack: implement get, set and remove acl hook
integrity: implement get and set acl hook
evm: add post set acl hook
internal: add may_write_xattr()
acl: add vfs_set_acl()
acl: add vfs_get_acl()
acl: add vfs_remove_acl()
ksmbd: use vfs_remove_acl()
ecryptfs: implement get acl method
ecryptfs: implement set acl method
ovl: implement get acl method
ovl: implement set acl method
ovl: use posix acl api
xattr: use posix acl api
evm: remove evm_xattr_acl_change()
ecryptfs: use stub posix acl handlers
ovl: use stub posix acl handlers
cifs: use stub posix acl handlers
9p: use stub posix acl handlers
acl: remove a slew of now unused helpers
Merge branch 'fs.acl.rework' into for-next
mnt_idmapping: add missing helpers
fs: use type safe idmapping helpers
caps: use type safe idmapping helpers
apparmor: use type safe idmapping helpers
ima: use type safe idmapping helpers
fuse: port to vfs{g,u}id_t and associated helpers
ovl: port to vfs{g,u}id_t and associated helpers
fs: remove unused idmapping helpers
Merge branch 'fs.vfsuid.conversion' into for-next
acl: make vfs_posix_acl_to_xattr() static
Merge branch 'fs.acl.rework' into for-next
cifs: check whether acl is valid early
Merge branch 'fs.acl.rework' into for-next
fs: introduce dedicated idmap type for mounts
acl: conver higher-level helpers to rely on mnt_idmap
Documentation/filesystems/locking.rst | 10 +-
Documentation/filesystems/porting.rst | 4 +-
Documentation/filesystems/vfs.rst | 5 +-
Documentation/trace/ftrace.rst | 2 +-
fs/9p/acl.c | 295 +++++++-------
fs/9p/acl.h | 8 +-
fs/9p/vfs_inode_dotl.c | 4 +
fs/9p/xattr.c | 7 +-
fs/9p/xattr.h | 2 -
fs/attr.c | 74 +++-
fs/bad_inode.c | 4 +-
fs/btrfs/acl.c | 3 +-
fs/btrfs/ctree.h | 2 +-
fs/btrfs/inode.c | 8 +-
fs/ceph/acl.c | 3 +-
fs/ceph/dir.c | 2 +-
fs/ceph/inode.c | 4 +-
fs/ceph/super.h | 2 +-
fs/cifs/cifsacl.c | 139 +++++++
fs/cifs/cifsfs.c | 4 +
fs/cifs/cifsproto.h | 20 +-
fs/cifs/cifssmb.c | 206 ++++++----
fs/cifs/xattr.c | 68 +---
fs/coredump.c | 4 +-
fs/ecryptfs/inode.c | 32 ++
fs/erofs/inode.c | 6 +-
fs/erofs/namei.c | 2 +-
fs/exec.c | 16 +-
fs/ext2/acl.c | 3 +-
fs/ext2/acl.h | 2 +-
fs/ext2/file.c | 2 +-
fs/ext2/inode.c | 2 +-
fs/ext2/namei.c | 4 +-
fs/ext4/acl.c | 3 +-
fs/ext4/acl.h | 2 +-
fs/ext4/file.c | 2 +-
fs/ext4/ialloc.c | 2 +-
fs/ext4/inode.c | 2 +-
fs/ext4/namei.c | 4 +-
fs/f2fs/acl.c | 4 +-
fs/f2fs/acl.h | 2 +-
fs/f2fs/file.c | 4 +-
fs/f2fs/namei.c | 4 +-
fs/fuse/acl.c | 5 +-
fs/fuse/dir.c | 4 +-
fs/fuse/file.c | 2 +-
fs/fuse/fuse_i.h | 2 +-
fs/gfs2/acl.c | 3 +-
fs/gfs2/acl.h | 2 +-
fs/gfs2/inode.c | 6 +-
fs/inode.c | 72 ++--
fs/internal.h | 35 +-
fs/jffs2/acl.c | 3 +-
fs/jffs2/acl.h | 2 +-
fs/jffs2/dir.c | 2 +-
fs/jffs2/file.c | 2 +-
fs/jffs2/fs.c | 2 +-
fs/jfs/acl.c | 3 +-
fs/jfs/file.c | 4 +-
fs/jfs/jfs_acl.h | 2 +-
fs/jfs/namei.c | 2 +-
fs/ksmbd/smb2pdu.c | 8 +-
fs/ksmbd/smbacl.c | 6 +-
fs/ksmbd/vfs.c | 21 +-
fs/ksmbd/vfs.h | 4 +-
fs/namei.c | 44 +--
fs/namespace.c | 176 +++++++--
fs/nfs/nfs3_fs.h | 2 +-
fs/nfs/nfs3acl.c | 9 +-
fs/nfs/nfs3proc.c | 4 +-
fs/nfsd/nfs2acl.c | 8 +-
fs/nfsd/nfs3acl.c | 8 +-
fs/nfsd/nfs4acl.c | 4 +-
fs/nfsd/vfs.c | 4 +-
fs/ntfs3/file.c | 4 +-
fs/ntfs3/namei.c | 4 +-
fs/ntfs3/ntfs_fs.h | 4 +-
fs/ntfs3/xattr.c | 9 +-
fs/ocfs2/acl.c | 3 +-
fs/ocfs2/acl.h | 2 +-
fs/ocfs2/file.c | 8 +-
fs/ocfs2/namei.c | 2 +-
fs/open.c | 8 +-
fs/orangefs/acl.c | 47 +--
fs/orangefs/inode.c | 47 ++-
fs/orangefs/namei.c | 2 +-
fs/orangefs/orangefs-kernel.h | 8 +-
fs/orangefs/orangefs-utils.c | 10 +-
fs/overlayfs/copy_up.c | 38 ++
fs/overlayfs/dir.c | 22 +-
fs/overlayfs/file.c | 28 +-
fs/overlayfs/inode.c | 187 +++++++--
fs/overlayfs/overlayfs.h | 42 +-
fs/overlayfs/super.c | 107 +----
fs/overlayfs/util.c | 9 +-
fs/posix_acl.c | 725 +++++++++++++++++-----------------
fs/reiserfs/acl.h | 6 +-
fs/reiserfs/file.c | 2 +-
fs/reiserfs/inode.c | 2 +-
fs/reiserfs/namei.c | 4 +-
fs/reiserfs/xattr_acl.c | 11 +-
fs/remap_range.c | 2 +-
fs/stat.c | 7 +-
fs/xattr.c | 116 +++---
fs/xfs/xfs_acl.c | 3 +-
fs/xfs/xfs_acl.h | 2 +-
fs/xfs/xfs_iops.c | 16 +-
include/linux/evm.h | 49 +++
include/linux/fs.h | 56 +--
include/linux/ima.h | 24 ++
include/linux/lsm_hook_defs.h | 6 +
include/linux/lsm_hooks.h | 12 +
include/linux/mnt_idmapping.h | 108 ++---
include/linux/mount.h | 9 +-
include/linux/posix_acl.h | 41 +-
include/linux/posix_acl_xattr.h | 47 ++-
include/linux/security.h | 29 ++
include/linux/xattr.h | 6 +
io_uring/xattr.c | 8 +-
kernel/capability.c | 4 +-
mm/shmem.c | 2 +-
security/apparmor/domain.c | 8 +-
security/apparmor/file.c | 4 +-
security/apparmor/lsm.c | 25 +-
security/commoncap.c | 51 +--
security/integrity/evm/evm_main.c | 147 ++++---
security/integrity/ima/ima_appraise.c | 9 +
security/integrity/ima/ima_policy.c | 34 +-
security/security.c | 42 ++
security/selinux/hooks.c | 22 ++
security/smack/smack_lsm.c | 71 ++++
131 files changed, 2291 insertions(+), 1439 deletions(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [GIT PULL] fs idmapped updates for v6.2
2022-12-12 13:19 [GIT PULL] fs idmapped updates for v6.2 Christian Brauner
@ 2022-12-13 3:40 ` Linus Torvalds
2022-12-13 9:40 ` Christian Brauner
2022-12-13 11:54 ` [PATCH] mnt_idmapping: move ima-only helpers to ima Christian Brauner
2022-12-13 3:49 ` [GIT PULL] fs idmapped updates for v6.2 pr-tracker-bot
1 sibling, 2 replies; 6+ messages in thread
From: Linus Torvalds @ 2022-12-13 3:40 UTC (permalink / raw)
To: Christian Brauner; +Cc: linux-fsdevel, linux-kernel
On Mon, Dec 12, 2022 at 5:19 AM Christian Brauner <brauner@kernel.org> wrote:
>
> Please note the tag contains all other branches for this cycle merged in.
Well, considering that the explanation basically assumed I had already
merged those (and I had), I wish you also had made the diffstat and
the shortlog reflect that.
As it was, now the diffstat and shortlog ends up containing not what
this last pull request brought in, but what they *all* brought in...
I'm also not super-happy with how ugly your history for this branch
was. You had literally merged the acl rework branch three times - at
different points of that branch.
Do we have other ugly history in the tree? Yes. But we've been getting
better. This was _not_ one of those "getting better" moments.
Oh well. I can see what you wanted to do, and I agree with the end
result, I just don't particularly like how this was done.
I've pulled it.
Linus
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [GIT PULL] fs idmapped updates for v6.2
2022-12-12 13:19 [GIT PULL] fs idmapped updates for v6.2 Christian Brauner
2022-12-13 3:40 ` Linus Torvalds
@ 2022-12-13 3:49 ` pr-tracker-bot
1 sibling, 0 replies; 6+ messages in thread
From: pr-tracker-bot @ 2022-12-13 3:49 UTC (permalink / raw)
To: Christian Brauner
Cc: Linus Torvalds, Christian Brauner, linux-fsdevel, linux-kernel
The pull request you sent on Mon, 12 Dec 2022 14:19:15 +0100:
> ssh://git@gitolite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.idmapped.mnt_idmap.v6.2
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/9b93f5069fd95cea7915aab321fd74d2548ba75c
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [GIT PULL] fs idmapped updates for v6.2
2022-12-13 3:40 ` Linus Torvalds
@ 2022-12-13 9:40 ` Christian Brauner
2022-12-13 11:54 ` [PATCH] mnt_idmapping: move ima-only helpers to ima Christian Brauner
1 sibling, 0 replies; 6+ messages in thread
From: Christian Brauner @ 2022-12-13 9:40 UTC (permalink / raw)
To: Linus Torvalds; +Cc: linux-fsdevel, linux-kernel
On Mon, Dec 12, 2022 at 07:40:29PM -0800, Linus Torvalds wrote:
> On Mon, Dec 12, 2022 at 5:19 AM Christian Brauner <brauner@kernel.org> wrote:
> >
> > Please note the tag contains all other branches for this cycle merged in.
>
> Well, considering that the explanation basically assumed I had already
> merged those (and I had), I wish you also had made the diffstat and
> the shortlog reflect that.
I wasn't sure what the best way was.
I'll make a note to use a better shortlog this time.
>
> As it was, now the diffstat and shortlog ends up containing not what
> this last pull request brought in, but what they *all* brought in...
I didn't want it to look like I was trying to hide the pretty obvious
ugliness of the branch by editing the shortlog.
>
> I'm also not super-happy with how ugly your history for this branch
> was. You had literally merged the acl rework branch three times - at
> different points of that branch.
I hate the history of that branch. And I have zero idea why I didn't
rebase when I applied it before I pushed it into linux-next.
I really had to fight the __very__ strong urge to rebase before sending
this pr. I had to step outside for a walk to resist it.
>
> Do we have other ugly history in the tree? Yes. But we've been getting
> better. This was _not_ one of those "getting better" moments.
>
> Oh well. I can see what you wanted to do, and I agree with the end
> result, I just don't particularly like how this was done.
>
> I've pulled it.
Thank you!
Christian
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] mnt_idmapping: move ima-only helpers to ima
2022-12-13 3:40 ` Linus Torvalds
2022-12-13 9:40 ` Christian Brauner
@ 2022-12-13 11:54 ` Christian Brauner
2022-12-13 20:21 ` Linus Torvalds
1 sibling, 1 reply; 6+ messages in thread
From: Christian Brauner @ 2022-12-13 11:54 UTC (permalink / raw)
To: Linus Torvalds; +Cc: Christian Brauner, linux-fsdevel
The vfs{g,u}id_{gt,lt}_* helpers are currently not needed outside of
ima and we shouldn't incentivize people to use them by placing them into
the header. Let's just define them locally in the one file in ima where
they are used.
Suggested-by: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Christian Brauner (Microsoft) <brauner@kernel.org>
---
include/linux/mnt_idmapping.h | 20 --------------------
security/integrity/ima/ima_policy.c | 24 ++++++++++++++++++++++++
2 files changed, 24 insertions(+), 20 deletions(-)
diff --git a/include/linux/mnt_idmapping.h b/include/linux/mnt_idmapping.h
index 092c52aa6c2c..0ccca33a7a6d 100644
--- a/include/linux/mnt_idmapping.h
+++ b/include/linux/mnt_idmapping.h
@@ -96,26 +96,6 @@ static inline bool vfsgid_eq_kgid(vfsgid_t vfsgid, kgid_t kgid)
return vfsgid_valid(vfsgid) && __vfsgid_val(vfsgid) == __kgid_val(kgid);
}
-static inline bool vfsuid_gt_kuid(vfsuid_t vfsuid, kuid_t kuid)
-{
- return __vfsuid_val(vfsuid) > __kuid_val(kuid);
-}
-
-static inline bool vfsgid_gt_kgid(vfsgid_t vfsgid, kgid_t kgid)
-{
- return __vfsgid_val(vfsgid) > __kgid_val(kgid);
-}
-
-static inline bool vfsuid_lt_kuid(vfsuid_t vfsuid, kuid_t kuid)
-{
- return __vfsuid_val(vfsuid) < __kuid_val(kuid);
-}
-
-static inline bool vfsgid_lt_kgid(vfsgid_t vfsgid, kgid_t kgid)
-{
- return __vfsgid_val(vfsgid) < __kgid_val(kgid);
-}
-
/*
* vfs{g,u}ids are created from k{g,u}ids.
* We don't allow them to be created from regular {u,g}id.
diff --git a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c
index 54c475f98ce1..edd95ba02c11 100644
--- a/security/integrity/ima/ima_policy.c
+++ b/security/integrity/ima/ima_policy.c
@@ -71,6 +71,30 @@ struct ima_rule_opt_list {
char *items[];
};
+/*
+ * These comparators are needed nowhere outside of ima so just define them here.
+ * This pattern should hopefully never be needed outside of ima.
+ */
+static inline bool vfsuid_gt_kuid(vfsuid_t vfsuid, kuid_t kuid)
+{
+ return __vfsuid_val(vfsuid) > __kuid_val(kuid);
+}
+
+static inline bool vfsgid_gt_kgid(vfsgid_t vfsgid, kgid_t kgid)
+{
+ return __vfsgid_val(vfsgid) > __kgid_val(kgid);
+}
+
+static inline bool vfsuid_lt_kuid(vfsuid_t vfsuid, kuid_t kuid)
+{
+ return __vfsuid_val(vfsuid) < __kuid_val(kuid);
+}
+
+static inline bool vfsgid_lt_kgid(vfsgid_t vfsgid, kgid_t kgid)
+{
+ return __vfsgid_val(vfsgid) < __kgid_val(kgid);
+}
+
struct ima_rule_entry {
struct list_head list;
int action;
base-commit: 764822972d64e7f3e6792278ecc7a3b3c81087cd
--
2.34.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] mnt_idmapping: move ima-only helpers to ima
2022-12-13 11:54 ` [PATCH] mnt_idmapping: move ima-only helpers to ima Christian Brauner
@ 2022-12-13 20:21 ` Linus Torvalds
0 siblings, 0 replies; 6+ messages in thread
From: Linus Torvalds @ 2022-12-13 20:21 UTC (permalink / raw)
To: Christian Brauner; +Cc: linux-fsdevel
On Tue, Dec 13, 2022 at 3:54 AM Christian Brauner <brauner@kernel.org> wrote:
>
> The vfs{g,u}id_{gt,lt}_* helpers are currently not needed outside of
> ima and we shouldn't incentivize people to use them by placing them into
> the header. Let's just define them locally in the one file in ima where
> they are used.
Thanks, LGTM.
Linus
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-12-13 20:27 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-12-12 13:19 [GIT PULL] fs idmapped updates for v6.2 Christian Brauner
2022-12-13 3:40 ` Linus Torvalds
2022-12-13 9:40 ` Christian Brauner
2022-12-13 11:54 ` [PATCH] mnt_idmapping: move ima-only helpers to ima Christian Brauner
2022-12-13 20:21 ` Linus Torvalds
2022-12-13 3:49 ` [GIT PULL] fs idmapped updates for v6.2 pr-tracker-bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.