Re: [PATCH v3 2/2] afs, bash: Fix open(O_CREAT) on an extant AFS file in a sticky dir

On Tue, Jul 28, 2026 at 11:14 AM David Howells <[email protected]> wrote:
>
> Since version 1.11 (January 1992) Bash has a work around in redir_open()
> that causes open(O_CREAT) of a file to be retried without O_CREAT if open()
> fails with an EACCES error if bash was built with AFS workarounds
> configured:
>
>         #if defined (AFS)
>               if ((fd < 0) && (errno == EACCES))
>             {
>               fd = open (filename, flags & ~O_CREAT, mode);
>               errno = EACCES;    /* restore errno */
>             }
>
>         #endif /* AFS */
>
> The ~O_CREAT fallback logic was introduced to workaround a bug[1] in the
> IBM AFS 3.1 cache manager and server which can return EACCES in preference
> to EEXIST if the requested file exists but the caller is neither granted
> explicit PRSFS_READ permission nor is the file owner and is granted
> PRSFS_INSERT permission on the directory.  IBM AFS 3.2 altered the cache
> manager permission checks but failed to correct the permission checks in
> the AFS server.  As of this writing, all IBM AFS derived servers continue
> to return EACCES in preference to EEXIST when these conditions are met.
> Bug reports have been filed with all implementations.
>
> As an unintended side effect, the Bash fallback logic also undermines the
> Linux kernel protections against O_CREAT opening FIFOs and regular files
> not owned by the user in world writeable sticky directories - unless the
> owner is the same as that of the directory - as was added in commit
> 30aba6656f61e ("namei: allow restricted O_CREAT of FIFOs and regular
> files").
>
> As a result the Bash fallback logic masks an incompatibility between the
> ownership checks performed by may_create_in_sticky() and network
> filesystems such as AFS where the uid namespace is disjoint from the uid
> namespace of the local system.
>
> However, the bash work around has been removed in bash-5.3[2].
>
> Fix this in the kernel by using a preceding patch that allows the user ID
> comparisons to be overridden by:
>
>  (1) Implement the ->is_owned_by_me() inode op for kafs to determine if the
>      caller owns the file by checking to see if the server indicated the
>      ADMINISTER bit was set in the access rights returned by the
>      FS.FetchStatus and suchlike instead of checking the i_uid to
>      current_fsuid().
>
>      Unfortunately, this check doesn't work for directories, but none of
>      the ops should require that.
>
>      Note that anonymous accesses to AFS will never see the ADMINISTER bit
>      being set and so will not be perceived as owning an anonymously-owned
>      file.
>
>  (2) Implement the ->have_same_owner() inode op, for kafs to compare the
>      AFS owner IDs retrieved by FS.FetchStatus (which are 64-bit integers
>      with AuriStor's YFS server and, as such, won't fit in a kuid_t).
>
>      Note that whilst an anonymously-owned file will match an
>      anonymously-owned parent directory, an anonymously-owned directory
>      cannot have the sticky bit set.
>
> This can be tested by creating a sticky directory (the user must have a
> token to do this) and creating a file in it.  Then strace bash doing "echo
> foo >>file" and look at whether bash does a single, successful O_CREAT open
> on the file or whether that one fails and then bash does one without
> O_CREAT that succeeds.
>
> Fixes: 30aba6656f61 ("namei: allow restricted O_CREAT of FIFOs and regular files")
> Reported-by: Etienne Champetier <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: Marc Dionne <[email protected]>
> cc: Jeffrey Altman <[email protected]>
> cc: Chet Ramey <[email protected]>
> cc: Cheyenne Wills <[email protected]>
> cc: Alexander Viro <[email protected]>
> cc: Christian Brauner <[email protected]>
> cc: Steve French <[email protected]>
> cc: [email protected]
> cc: [email protected]
> cc: [email protected]
> cc: [email protected]
> Link: https://groups.google.com/g/gnu.bash.bug/c/6PPTfOgFdL4/m/2AQU-S1N76UJ [1]
> Link: https://git.savannah.gnu.org/cgit/bash.git/tree/redir.c?h=bash-5.3-rc1#n733 [2]
> ---
>  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 ++
>  6 files changed, 56 insertions(+)
>
> diff --git a/fs/afs/dir.c b/fs/afs/dir.c
> index 6df56fe9163f..ee2f5ec0b2c6 100644
> --- a/fs/afs/dir.c
> +++ b/fs/afs/dir.c
> @@ -69,6 +69,8 @@ const struct inode_operations afs_dir_inode_operations = {
>         .permission     = afs_permission,
>         .getattr        = afs_getattr,
>         .setattr        = afs_setattr,
> +       .is_owned_by_me = afs_is_owned_by_me,
> +       .have_same_owner = afs_have_same_owner,
>  };
>
>  const struct address_space_operations afs_dir_aops = {
> diff --git a/fs/afs/file.c b/fs/afs/file.c
> index 0467742bfeee..ba44493ccaf6 100644
> --- a/fs/afs/file.c
> +++ b/fs/afs/file.c
> @@ -49,6 +49,8 @@ const struct inode_operations afs_file_inode_operations = {
>         .getattr        = afs_getattr,
>         .setattr        = afs_setattr,
>         .permission     = afs_permission,
> +       .is_owned_by_me = afs_is_owned_by_me,
> +       .have_same_owner = afs_have_same_owner,
>  };
>
>  const struct address_space_operations afs_file_aops = {
> diff --git a/fs/afs/inode.c b/fs/afs/inode.c
> index 14f39a9bea6c..af85a88d943d 100644
> --- a/fs/afs/inode.c
> +++ b/fs/afs/inode.c
> @@ -91,6 +91,7 @@ static int afs_inode_init_from_status(struct afs_operation *op,
>         inode->i_flags |= S_NOATIME;
>         inode->i_uid = make_kuid(&init_user_ns, status->owner);
>         inode->i_gid = make_kgid(&init_user_ns, status->group);
> +       inode->i_opflags |= IOP_OWNERSHIP_OVERRIDE;
>         set_nlink(&vnode->netfs.inode, status->nlink);
>
>         i_size_write(inode, status->size);
> diff --git a/fs/afs/internal.h b/fs/afs/internal.h
> index 290873bac89b..f15dbbc8dd26 100644
> --- a/fs/afs/internal.h
> +++ b/fs/afs/internal.h
> @@ -1531,6 +1531,9 @@ extern struct key *afs_request_key(struct afs_cell *);
>  extern struct key *afs_request_key_rcu(struct afs_cell *);
>  extern int afs_check_permit(struct afs_vnode *, struct key *, afs_access_t *);
>  extern int afs_permission(struct mnt_idmap *, struct inode *, int);
> +int afs_is_owned_by_me(struct mnt_idmap *idmap, struct inode *inode);
> +int afs_have_same_owner(struct mnt_idmap *idmap, struct inode *inode1,
> +                       struct inode *inode2);
>  extern void __exit afs_clean_up_permit_cache(void);
>
>  /*
> diff --git a/fs/afs/security.c b/fs/afs/security.c
> index 6d00d62a65ed..7092f5acdcd6 100644
> --- a/fs/afs/security.c
> +++ b/fs/afs/security.c
> @@ -510,6 +510,52 @@ int afs_permission(struct mnt_idmap *idmap, struct inode *inode,
>         return ret;
>  }
>
> +/*
> + * Determine if an inode is owned by 'me' - whatever that means for the
> + * filesystem.  In the case of AFS, this means that the file is owned by the
> + * AFS user represented by the Rx Security Class token held in a key.  Returns
> + * 0 if owned by me, 1 if not; can also return an error.
> + */
> +int afs_is_owned_by_me(struct mnt_idmap *idmap, struct inode *inode)
> +{
> +       struct afs_vnode *vnode = AFS_FS_I(inode);
> +       afs_access_t access;
> +       struct key *key;
> +       int ret;
> +
> +       if (S_ISDIR(inode->i_mode))
> +               return 1; /* The ADMIN right check doesn't work for directories. */
> +
> +       key = afs_request_key(vnode->volume->cell);
> +       if (IS_ERR(key))
> +               return PTR_ERR(key);
> +
> +       /* Get the access rights for the key on this file. */
> +       ret = afs_check_permit(vnode, key, &access);
> +       if (ret < 0)
> +               goto error;
> +
> +       /* We get the ADMINISTER bit if we own the file. */
> +       ret = (access & AFS_ACE_ADMINISTER) ? 0 : 1;
> +error:
> +       key_put(key);
> +       return ret;
> +}
> +
> +/*
> + * Determine if a file has the same owner as its parent - whatever that means
> + * for the filesystem.  In the case of AFS, this means comparing their AFS
> + * UIDs.  Returns 0 if same, 1 if not same; can also return an error.
> + */
> +int afs_have_same_owner(struct mnt_idmap *idmap, struct inode *inode1,
> +                       struct inode *inode2)
> +{
> +       const struct afs_vnode *vnode1 = AFS_FS_I(inode1);
> +       const struct afs_vnode *vnode2 = AFS_FS_I(inode2);
> +
> +       return vnode1->status.owner != vnode2->status.owner;
> +}
> +
>  void __exit afs_clean_up_permit_cache(void)
>  {
>         int i;
> diff --git a/fs/afs/symlink.c b/fs/afs/symlink.c
> index 16b4823cb7b7..ed834ba90e85 100644
> --- a/fs/afs/symlink.c
> +++ b/fs/afs/symlink.c
> @@ -271,6 +271,8 @@ int afs_symlink_writepages(struct address_space *mapping,
>  const struct inode_operations afs_symlink_inode_operations = {
>         .get_link       = afs_get_link,
>         .readlink       = afs_readlink,
> +       .is_owned_by_me = afs_is_owned_by_me,
> +       .have_same_owner = afs_have_same_owner,
>  };
>
>  const struct address_space_operations afs_symlink_aops = {
>
>

Acked-by: Marc Dionne <[email protected]>

Marc