Skip to content

vfs, afs, bash: Fix miscomparison of foreign user IDs in the VFS

David Howells linux-cifs linux-fsdevel linux-integrity linux-nfs lkml 4 versions – New

Get series with b4
b4 am [email protected]

Versions newest first · linked by author and title

  1. v4 Latest2 patches 0 repliesNew
  2. v32 patches 5 repliesSuperseded
  3. v22 patches 1 replySuperseded
Earlier versions · 1
VersionPostedPatchesRepliesStatus
v12 patches4 repliesSuperseded

Patches in v4 No applies recorded yet

#Subject and filesSizeRepliesApplied
1/2vfs: Allow filesystems with foreign owner IDs to override UID checksDocumentation/filesystems/vfs.rst fs/attr.c fs/coredump.c +6 more+217 -710 replies
2/2afs, bash: Fix open(O_CREAT) on an extant AFS file in a sticky dirfs/afs/dir.c fs/afs/file.c fs/afs/inode.c +3 more+56 -00 replies

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v4 yet.

Read the v4 thread

Cover letter [PATCH v4 0/2] vfs, afs, bash: Fix miscomparison of foreign user IDs in the VFS

Hi Al, Christian,

Here's a pair of fixes that deal with some places the VFS mishandles foreign user ID checks. By "foreign" I mean that the user IDs from the filesystem do not belong in the same number space as the system's user IDs. Network filesystems are prime examples of this, but it may also impact things like USB drives or cdroms.

Take AFS as example: Whilst each file does have a numeric user ID, the file may be accessed from a world-accessible public-facing server from some other organisation with its own idea of what that user ID refers to. IDs from AFS may also collide with the system's own set of IDs and may also be unrepresentable as a 32-bit UID (in the case of AuriStor servers).

Further, kAFS uses a key containing an authentication token to specify the subject doing an RPC operation to the server - and, as such, this needs to be used instead of current_fsuid() in determining whether the current user has ownership rights over a file.

Additionally, filesystems (CIFS being a notable example) may also have user identifiers that aren't simple integers.

Now the problem in the VFS is that there are a number of places where it assumes it can directly compare i_uid (possibly id-mapped) to either than on another inode or a UID drawn from elsewhere (e.g. current_uid()) - but this doesn't work right.

This causes the write-to-sticky check to work incorrectly for AFS (though this is currently masked by a workaround in bash that has now been removed as of bash-5.3) whereby open(O_CREAT) of such a file will fail when it shouldn't.

Two patches are provided:

(1) Add a pair of inode operations, one to compare the ownership of a pair of inodes and the other to see if the current process has ownership rights over an inode with an IOP flag to control access to them.

Usage of this is then extended out into the VFS, replacing comparisons between i_uid and i_uid and between i_uid and current_fsuid(). The default, it the inode ops are unimplemented, is to do those direct i_uid comparisons.

(2) Fixes the bash workaround issue with regard to AFS, overriding the checks as to whether two inodes have the same owner and the check as to whether the current user owns an inode to work within the AFS model.

kAFS uses the result of a status-fetch with a suitable key to determine file ownership (if the ADMINISTER bit is set) and just compares the 64-bit owner IDs to determine if two inodes have the same ownership.

Note that chown may also need modifying in some way - but the system call as it stands can't necessarily supply the information required (for instance, an AuriStor YFS ID is 64 bits, but chown can only handle a 32-bit integer; CIFS might use a GUID).

The patches can be found here:

https://git.kernel.org/pub/scm/linux/kernel/git/dhowells/linux-fs.git/log/?h=afs-sticky-2

Thanks,
David

Changes
=======
ver #4)
- Rebased on v7.3-rc1.

ver #3)
- Fixed afs patch to provide ops for afs symlinks too.

ver #2)
- Rebased on v7.2-rc4.
- Added an IOP_ flag for a faster check of whether the new inode ops are supported.

David Howells (2):
vfs: Allow filesystems with foreign owner IDs to override UID checks afs, bash: Fix open(O_CREAT) on an extant AFS file in a sticky dir

 Documentation/filesystems/vfs.rst |  21 ++++
 fs/afs/dir.c                      |   2 +
 fs/afs/file.c                     |   2 +
 fs/afs/inode.c                    |   1 +
 fs/afs/internal.h                 |   3 +
 fs/afs/security.c                 |  46 +++++++++
 fs/afs/symlink.c                  |   2 +
 fs/attr.c                         |  58 ++++++-----
 fs/coredump.c                     |   2 +-
 fs/inode.c                        |  11 +-
 fs/internal.h                     |   1 +
 fs/locks.c                        |   7 +-
 fs/namei.c                        | 161 ++++++++++++++++++++++++------
 fs/remap_range.c                  |  20 ++--
 include/linux/fs.h                |   7 +-
 15 files changed, 273 insertions(+), 71 deletions(-)