fs.fixes.v6.0-rc7

-----BEGIN PGP SIGNATURE-----
 
 iHUEABYKAB0WIQRAhzRXHqcMeLMyaSiRxhvAZXjcogUCYymPoQAKCRCRxhvAZXjc
 ounZAQDGLmHjqby6KFLbNIHkgIMzODUk3OCLo3jNRsSw+SsJFQD/cW1eBM5P+ctO
 bePiCHMZv4Gh+G1dR2cchd3Etwks4A0=
 =7kI/
 -----END PGP SIGNATURE-----

Merge tag 'fs.fixes.v6.0-rc7' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping

Pull vfs fix from Christian Brauner:
 "Beginning of the merge window we introduced the vfs{g,u}id_t types in
  b27c82e129 ("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 3cb6ee9914 ("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)"

* tag 'fs.fixes.v6.0-rc7' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/idmapping:
  open: always initialize ownership fields

fs/open.c

@@ -716,6 +716,8 @@ int chown_common(const struct path *path, uid_t user, gid_t group)
 	fs_userns = i_user_ns(inode);
 
 retry_deleg:
+	newattrs.ia_vfsuid = INVALID_VFSUID;
+	newattrs.ia_vfsgid = INVALID_VFSGID;
 	newattrs.ia_valid =  ATTR_CTIME;
 	if ((user != (uid_t)-1) && !setattr_vfsuid(&newattrs, uid))
 		return -EINVAL;