> On Apr 14, 2022, at 11:49 AM, Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
>
> From: Darrick J. Wong <djwong@xxxxxxxxxx>
>
> Catherine's recent changes to xfs/019 exposed a bug in how libxfs
> handles setgid bits. mkfs reads the desired gid in from the protofile,
> but if the parent directory is setgid, it will override the user's
> setting and (re)set the child's gid to the parent's gid. Overriding
> user settings is (probably) not the desired mode of operation, so create
> a flag to struct cred to force the gid in the protofile.
>
> It looks like this has been broken since ~2005.
Looks good to me
Reviewed-by: Catherine Hoang <catherine.hoang@xxxxxxxxxx>
>
> Cc: Catherine Hoang <catherine.hoang@xxxxxxxxxx>
> Fixes: 9f064b7e ("Provide mkfs options to easily exercise all inheritable attributes, esp. the extsize allocator hint. Merge of master-melb:xfs-cmds:24370a by kenmcd.")
> Signed-off-by: Darrick J. Wong <djwong@xxxxxxxxxx>
> ---
> include/xfs_inode.h | 11 +++++++----
> libxfs/util.c | 3 ++-
> mkfs/proto.c | 3 ++-
> 3 files changed, 11 insertions(+), 6 deletions(-)
>
>
> diff --git a/include/xfs_inode.h b/include/xfs_inode.h
> index 08a62d83..db9faa57 100644
> --- a/include/xfs_inode.h
> +++ b/include/xfs_inode.h
> @@ -164,10 +164,13 @@ static inline bool xfs_inode_has_bigtime(struct xfs_inode *ip)
> return ip->i_diflags2 & XFS_DIFLAG2_BIGTIME;
> }
>
> -typedef struct cred {
> - uid_t cr_uid;
> - gid_t cr_gid;
> -} cred_t;
> +/* Always set the child's GID to this value, even if the parent is setgid. */
> +#define CRED_FORCE_GID (1U << 0)
> +struct cred {
> + uid_t cr_uid;
> + gid_t cr_gid;
> + unsigned int cr_flags;
> +};
>
> extern int libxfs_dir_ialloc (struct xfs_trans **, struct xfs_inode *,
> mode_t, nlink_t, xfs_dev_t, struct cred *,
> diff --git a/libxfs/util.c b/libxfs/util.c
> index 9f1ca907..655a7bd3 100644
> --- a/libxfs/util.c
> +++ b/libxfs/util.c
> @@ -271,7 +271,8 @@ libxfs_init_new_inode(
> xfs_trans_ichgtime(tp, ip, XFS_ICHGTIME_CHG | XFS_ICHGTIME_MOD);
>
> if (pip && (VFS_I(pip)->i_mode & S_ISGID)) {
> - VFS_I(ip)->i_gid = VFS_I(pip)->i_gid;
> + if (!(cr->cr_flags & CRED_FORCE_GID))
> + VFS_I(ip)->i_gid = VFS_I(pip)->i_gid;
> if ((VFS_I(pip)->i_mode & S_ISGID) && (mode & S_IFMT) == S_IFDIR)
> VFS_I(ip)->i_mode |= S_ISGID;
> }
> diff --git a/mkfs/proto.c b/mkfs/proto.c
> index ef130ed6..127d87dd 100644
> --- a/mkfs/proto.c
> +++ b/mkfs/proto.c
> @@ -378,7 +378,7 @@ parseproto(
> xfs_trans_t *tp;
> int val;
> int isroot = 0;
> - cred_t creds;
> + struct cred creds;
> char *value;
> struct xfs_name xname;
>
> @@ -446,6 +446,7 @@ parseproto(
> mode |= val;
> creds.cr_uid = (int)getnum(getstr(pp), 0, 0, false);
> creds.cr_gid = (int)getnum(getstr(pp), 0, 0, false);
> + creds.cr_flags = CRED_FORCE_GID;
> xname.name = (unsigned char *)name;
> xname.len = name ? strlen(name) : 0;
> xname.type = 0;
>
