Hey Linus,
/* Summary */
Last cycle we introduced the vfs{g,u}id_t types and associated helpers to gain
type safety when dealing with idmapped mounts. That initial pull request back
then already converted a lot of places over but there were still some left,
This pull request converts all remaining places that still make use of non-type
safe idmapping helpers to rely on the new type safe vfs{g,u}id based helpers.
Afterwards it removes all the old non-type safe helpers.
Note that this pull request has the setgid inheritance branch merged in as the
setgid inheritance branch unifies multiple open-coded checks into a single
helper making the conversion here easier. I've sent a pull request for that
work rearlier so it's on the list and in your inbox before this one. The lore
url is:
https://lore.kernel.org/lkml/[email protected]
In case you don't want to pull "setgid inheritance updates for v6.2" but still
would like to pull the remaining vfs{g,u}id_t conversions (That would be
greatly appreciated as it gets rid of duplicated functionality between the
different helpers.) I prepared the tag
fs.vfsuid.conversion.standalone.v6.2
This tag only contains all the vfs{g,u}id_t patches without any of the "setgid
inheritance updates for v6.2" patches.
ssh://[email protected]/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.vfsuid.conversion.standalone.v6.2
/* 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. The vfsuid conversionn portion passes 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.
/* 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://[email protected]/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.vfsuid.conversion.v6.2
__Alternatively__, a standalone version without the setgid patches merged in
can be found at:
ssh://[email protected]/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.vfsuid.conversion.standalone.v6.2
for you to fetch changes up to eb7718cdb73c6b0c93002f8f73f4dd4701f8d2bb:
fs: remove unused idmapping helpers (2022-10-26 10:03:34 +0200)
Please consider pulling these changes from the signed fs.vfsuid.conversion.v6.2
or fs.vfsuid.conversion.standalone.v6.2 tag.
Thanks!
Christian
----------------------------------------------------------------
fs.vfsuid.conversion.v6.2
----------------------------------------------------------------
Amir Goldstein (2):
ovl: remove privs in ovl_copyfile()
ovl: remove privs in ovl_fallocate()
Christian Brauner (12):
attr: add in_group_or_capable()
fs: move should_remove_suid()
attr: add setattr_should_drop_sgid()
attr: use consistent sgid stripping checks
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
Documentation/trace/ftrace.rst | 2 +-
fs/attr.c | 74 +++++++++++++++++++++++---
fs/coredump.c | 4 +-
fs/exec.c | 16 +++---
fs/fuse/acl.c | 2 +-
fs/fuse/file.c | 2 +-
fs/inode.c | 72 ++++++++++++--------------
fs/internal.h | 10 +++-
fs/namei.c | 40 +++++++--------
fs/ocfs2/file.c | 4 +-
fs/open.c | 8 +--
fs/overlayfs/file.c | 28 ++++++++--
fs/overlayfs/util.c | 9 +++-
fs/remap_range.c | 2 +-
fs/stat.c | 7 ++-
include/linux/fs.h | 36 +------------
include/linux/mnt_idmapping.h | 100 ++++++++++++------------------------
kernel/capability.c | 4 +-
security/apparmor/domain.c | 8 +--
security/apparmor/file.c | 4 +-
security/apparmor/lsm.c | 25 ++++++---
security/commoncap.c | 51 +++++++++---------
security/integrity/ima/ima_policy.c | 34 ++++++------
23 files changed, 289 insertions(+), 253 deletions(-)
On Mon, Dec 12, 2022 at 4:34 AM Christian Brauner <[email protected]> wrote:
>
> This pull request converts all remaining places that still make use of non-type
> safe idmapping helpers to rely on the new type safe vfs{g,u}id based helpers.
> Afterwards it removes all the old non-type safe helpers.
So I've pulled this, but I'm not entirely happy about some of those
crazy helpers.
In particular, the whole "ordering" helpers are really not something
that should be used in general, I feel. I'm talking about
vfsuid_gt_kuid() and friends - it's an entirely insane operation and
makes no sense at all.
Yes, yes, I understand why they exist (those crazy IMA rules), but I
feel that those functions *really* shouldn't be exposed to anybody
else.
IOW, making those insane functions available in <linux/idmapping.h>
really seems wrong to me. They are crazy special cases, and I think
they should exist purely in that crazy ima_security file.
Again - I've pulled this, but I'm hoping to see a future commit that
limits that craziness to the only user, in the hope that this disease
will never spread.
Linus
The pull request you sent on Mon, 12 Dec 2022 13:33:48 +0100:
> ssh://[email protected]/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.vfsuid.conversion.standalone.v6.2
has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/e4236f97688afc21151bfc050acfce9ac3b56f6b
Thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/prtracker.html
On Mon, Dec 12, 2022 at 07:28:59PM -0800, Linus Torvalds wrote:
> On Mon, Dec 12, 2022 at 4:34 AM Christian Brauner <[email protected]> wrote:
> >
> > This pull request converts all remaining places that still make use of non-type
> > safe idmapping helpers to rely on the new type safe vfs{g,u}id based helpers.
> > Afterwards it removes all the old non-type safe helpers.
>
> So I've pulled this, but I'm not entirely happy about some of those
> crazy helpers.
>
> In particular, the whole "ordering" helpers are really not something
> that should be used in general, I feel. I'm talking about
> vfsuid_gt_kuid() and friends - it's an entirely insane operation and
> makes no sense at all.
Oh yes, I very much agree.
>
> Yes, yes, I understand why they exist (those crazy IMA rules), but I
I would've really liked to have avoided their existence altogether but I
have no clear idea what ima is doing with these comparisons. And
everytime we do wider scoped vfs work I spend about 1 or 2 good weeks in
security/ just to understand what all the various security modules do,
audit callchains and then come up with something that doesn't break half
of them. And often this means unpleasant compromises in the vfs layer
which I really don't like.
And just to be clear, I don't want to be on of those "LSMs are bad"
people. I do really think they provide additional value.
But I think it's fair to acknowledge that the hook infrastructure with
multiple LSMs makes the vfs and developers pay when reworking codepaths.
And the fact that some things that are LSM-like (ima etc.) have separate
hooks doesn't help either.
> feel that those functions *really* shouldn't be exposed to anybody
> else.
>
> IOW, making those insane functions available in <linux/idmapping.h>
> really seems wrong to me. They are crazy special cases, and I think
> they should exist purely in that crazy ima_security file.
>
> Again - I've pulled this, but I'm hoping to see a future commit that
> limits that craziness to the only user, in the hope that this disease
> will never spread.
Let me see what I can do about this. Hopefully I can still find
something during the merge window.
On 13/12/2022 04.28, Linus Torvalds wrote:
> On Mon, Dec 12, 2022 at 4:34 AM Christian Brauner <[email protected]> wrote:
>>
>> This pull request converts all remaining places that still make use of non-type
>> safe idmapping helpers to rely on the new type safe vfs{g,u}id based helpers.
>> Afterwards it removes all the old non-type safe helpers.
>
> So I've pulled this, but I'm not entirely happy about some of those
> crazy helpers.
>
> In particular, the whole "ordering" helpers are really not something
> that should be used in general, I feel. I'm talking about
> vfsuid_gt_kuid() and friends - it's an entirely insane operation and
> makes no sense at all.
>
> Yes, yes, I understand why they exist (those crazy IMA rules), but I
> feel that those functions *really* shouldn't be exposed to anybody
> else.
>
> IOW, making those insane functions available in <linux/idmapping.h>
> really seems wrong to me. They are crazy special cases, and I think
> they should exist purely in that crazy ima_security file.
Yeah. Aside from assigning any semantics to < or > of [ug]ids, which is
something IMA apparently wants to do, taking the address of a static
inline in the first place is a code smell; that obviously forces the
compiler to emit a copy in the current TU. But the code compares stored
pointers to addresses of those static inlines, which would be completely
broken if this didn't happen to all be contained in a single TU. That's
quite subtle, and probably fowner_op would be better as an enum.
Rasmus