Message-Id: <20220920105109.1315345-1-brauner@kernel.org>
Date:   Tue, 20 Sep 2022 12:51:10 +0200
From:   Christian Brauner <brauner@...nel.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Christian Brauner <brauner@...nel.org>,
        linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: [GIT PULL] fs fixes for v6.0-rc7

Hey Linus,

/* Summary */
Beginning of the merge window we introduced the vfs{g,u}id_t types in
b27c82e12965 ("attr: port attribute changes to new types") and changed various
codepaths over including chown_common().

When userspace passes -1 for an ownership change the ownership fields in struct
iattr stay uninitialized. Usually this is fine because any code making use of
any fields in struct iattr must check the ->ia_valid field whether the value of
interest has been initialized. That's true for all struct iattr passing code.

However, over the course of the last year with more heavy use of KMSAN we found
quite a few places that got this wrong. A recent one I fixed was 3cb6ee991496
("9p: only copy valid iattrs in 9P2000.L setattr implementation").

But we also have LSM hooks. Actually we have two. The first one is
security_inode_setattr() in notify_change() which does the right thing and
passes the full struct iattr down to LSMs and thus LSMs can check whether it is
initialized.

But then we also have security_path_chown() which passes down a path argument
and the target ownership as the filesystem would see it. For the latter we now
generate the target values based on struct iattr and pass it down. However,
when userspace passes -1 then struct iattr isn't initialized. This patch simply
initializes ->ia_vfs{g,u}id with INVALID_VFS{G,U}ID so the hook continue to see
invalid ownership when -1 is passed from userspace. The only LSM that cares
about the actual values is Tomoyo.

The vfs codepaths don't look at these fields without ->ia_valid being set so
there's no harm in initializing ->ia_vfs{g,u}id. Arguably this is also safer
since we can't end up copying valid ownership values when invalid ownership
values should be passed.

This only affects mainline. No kernel has been released with this and thus no
backport is needed. The commit is thus marked with a Fixes: tag but annotated
with "# mainline only" (I didn't quite remember what Greg said about how to
tell stable autoselect to not bother with fixes for mainline only.).

/* Testing */
All patches are based on v6.0-rc3. No build failures or warnings were observed
and fstests, selftests, and LTP have seen no 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 b90cb1053190353cc30f0fef0ef1f378ccc063c5:

  Linux 6.0-rc3 (2022-08-28 15:05:29 -0700)

are available in the Git repository at:

  ssh://git@...olite.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping.git tags/fs.fixes.v6.0-rc7

for you to fetch changes up to f52d74b190f8d10ec01cd5774eca77c2186c8ab7:

  open: always initialize ownership fields (2022-09-20 11:57:57 +0200)

Please consider pulling these changes from the signed fs.fixes.v6.0-rc7 tag.

Thanks!
Christian

----------------------------------------------------------------
fs.fixes.v6.0-rc7

----------------------------------------------------------------
Tetsuo Handa (1):
      open: always initialize ownership fields

 fs/open.c | 2 ++
 1 file changed, 2 insertions(+)

Powered by blists - more mailing lists