[PVFS2-developers] patch: setgid bit support
Rob Ross
rross at mcs.anl.gov
Thu Jun 30 17:54:44 EDT 2005
Looks good; thanks! It would be helpful to me if you would mention in
this kind of patch if the patch results in any additional messaging. In
this case it looks like the answer is no.
Regards,
Rob
Phil Carns wrote:
> setgid.patch:
>
> This patch adds setgid support to pvfs2 (server side, kernel module, and
> admin utitilies), including the semantics of propigating the setgid bit
> to new subdirectories or files. It also makes sure that setattr returns
> an error if the user requests permission bits that are not supported
> (just the setuid and sticky bits after this patch).
>
> -Phil
>
>
> ------------------------------------------------------------------------
>
> ---------------------
> PatchSet 33
> Date: 2005/03/08 23:37:25
> Author: pcarns
> Branch: HEAD
> Tag: (none)
> Log:
> - first step towards setgid support
> - file system now sets, unsets, and reports setgid in listings
> - no impact on policy yet
> [associate:task2939]
>
> Members:
> include/pvfs2-types.h:1.1->1.2
> include/pvfs2-util.h:1.1->1.2
> src/apps/kernel/linux/pvfs2-client-core.c:1.3->1.4
> src/kernel/linux-2.6/pvfs2-utils.c:1.2->1.3
>
> Index: include/pvfs2-types.h
> diff -u include/pvfs2-types.h:1.1 include/pvfs2-types.h:1.2
> --- include/pvfs2-types.h:1.1 Mon Feb 7 12:58:47 2005
> +++ include/pvfs2-types.h Tue Mar 8 16:37:25 2005
> @@ -142,4 +157,5 @@
> #define PVFS_U_EXECUTE (1 << 6)
> #define PVFS_U_WRITE (1 << 7)
> #define PVFS_U_READ (1 << 8)
> +#define PVFS_G_SGID (1 << 9)
>
> Index: include/pvfs2-util.h
> diff -u include/pvfs2-util.h:1.1 include/pvfs2-util.h:1.2
> --- include/pvfs2-util.h:1.1 Mon Feb 7 12:58:47 2005
> +++ include/pvfs2-util.h Tue Mar 8 16:37:25 2005
> @@ -128,20 +128,22 @@
> static inline int PVFS2_translate_mode(int mode)
> {
> int ret = 0, i = 0;
> - static int modes[9] =
> + static int modes[10] =
> {
> S_IXOTH, S_IWOTH, S_IROTH,
> S_IXGRP, S_IWGRP, S_IRGRP,
> - S_IXUSR, S_IWUSR, S_IRUSR
> + S_IXUSR, S_IWUSR, S_IRUSR,
> + S_ISGID
> };
> - static int pvfs2_modes[9] =
> + static int pvfs2_modes[10] =
> {
> PVFS_O_EXECUTE, PVFS_O_WRITE, PVFS_O_READ,
> PVFS_G_EXECUTE, PVFS_G_WRITE, PVFS_G_READ,
> PVFS_U_EXECUTE, PVFS_U_WRITE, PVFS_U_READ,
> + PVFS_G_SGID
> };
>
> - for(i = 0; i < 9; i++)
> + for(i = 0; i < 10; i++)
> {
> if (mode & modes[i])
> {
> Index: src/apps/kernel/linux/pvfs2-client-core.c
> diff -u src/apps/kernel/linux/pvfs2-client-core.c:1.3 src/apps/kernel/linux/pvfs2-client-core.c:1.4
> --- src/apps/kernel/linux/pvfs2-client-core.c:1.3 Wed Mar 2 13:53:35 2005
> +++ src/apps/kernel/linux/pvfs2-client-core.c Tue Mar 8 16:37:28 2005
> @@ -524,6 +524,9 @@
> "got a setattr request for fsid %d | handle %Lu\n",
> vfs_request->in_upcall.req.setattr.refn.fs_id,
> Lu(vfs_request->in_upcall.req.setattr.refn.handle));
> + gossip_debug(
> + GOSSIP_CLIENTCORE_DEBUG,
> + "perms: %d\n", (int)vfs_request->in_upcall.req.setattr.attributes.perms);
>
> ret = PVFS_isys_setattr(
> vfs_request->in_upcall.req.setattr.refn,
> Index: src/kernel/linux-2.6/pvfs2-utils.c
> diff -u src/kernel/linux-2.6/pvfs2-utils.c:1.2 src/kernel/linux-2.6/pvfs2-utils.c:1.3
> --- src/kernel/linux-2.6/pvfs2-utils.c:1.2 Fri Mar 4 16:25:37 2005
> +++ src/kernel/linux-2.6/pvfs2-utils.c Tue Mar 8 16:37:31 2005
> @@ -157,6 +157,9 @@
> if (attrs->perms & PVFS_U_READ)
> perm_mode |= S_IRUSR;
>
> + if (attrs->perms & PVFS_G_SGID)
> + perm_mode |= S_ISGID;
> +
> inode->i_mode |= perm_mode;
>
> switch (attrs->objtype)
> ---------------------
> PatchSet 34
> Date: 2005/03/08 23:49:34
> Author: pcarns
> Branch: HEAD
> Tag: (none)
> Log:
> make pvfs2-ls acknowledge setgid bit when listing with details
> [associate:task2939]
>
> Members:
> src/apps/admin/pvfs2-ls.c:1.1->1.2
>
> Index: src/apps/admin/pvfs2-ls.c
> diff -u src/apps/admin/pvfs2-ls.c:1.1 src/apps/admin/pvfs2-ls.c:1.2
> --- src/apps/admin/pvfs2-ls.c:1.1 Mon Feb 7 12:57:13 2005
> +++ src/apps/admin/pvfs2-ls.c Tue Mar 8 16:49:34 2005
> @@ -190,6 +190,7 @@
> char scratch_owner[16] = {0}, scratch_group[16] = {0};
> char scratch_size[16] = {0}, scratch_inode[16] = {0};
> char f_type = '-';
> + char group_x_char = '-';
>
> if (!opts->list_all && (entry_name[0] == '.'))
> {
> @@ -271,6 +272,16 @@
> f_type = 'l';
> }
>
> + /* special case to set setgid display for groups if needed */
> + if(attr->perms & PVFS_G_SGID)
> + {
> + group_x_char = ((attr->perms & PVFS_G_EXECUTE) ? 's' : 'S');
> + }
> + else
> + {
> + group_x_char = ((attr->perms & PVFS_G_EXECUTE) ? 'x' : '-');
> + }
> +
> snprintf(buf,128,"%s%c%c%c%c%c%c%c%c%c%c 1 %s %s %s "
> "%.4d-%.2d-%.2d %.2d:%.2d %s",
> inode,
> @@ -280,7 +291,7 @@
> ((attr->perms & PVFS_U_EXECUTE) ? 'x' : '-'),
> ((attr->perms & PVFS_G_READ) ? 'r' : '-'),
> ((attr->perms & PVFS_G_WRITE) ? 'w' : '-'),
> - ((attr->perms & PVFS_G_EXECUTE) ? 'x' : '-'),
> + group_x_char,
> ((attr->perms & PVFS_O_READ) ? 'r' : '-'),
> ((attr->perms & PVFS_O_WRITE) ? 'w' : '-'),
> ((attr->perms & PVFS_O_EXECUTE) ? 'x' : '-'),
> ---------------------
> PatchSet 45
> Date: 2005/03/17 20:40:08
> Author: pcarns
> Branch: HEAD
> Tag: (none)
> Log:
> more progress on setgid bit:
> - now honored correctly on file creation for both kernel and user space tools
> - updated pvfs2-chmod tool to support
> - corrected numeric value for SGID bit to better match common usage
> NOTE: still need to honor correctly on mkdir
> [associate:task2939]
>
> Members:
> include/pvfs2-types.h:1.2->1.3
> src/apps/admin/pvfs2-chmod.c:1.1->1.2
> src/client/sysint/sys-create.sm:1.1->1.2
>
> Index: include/pvfs2-types.h
> diff -u include/pvfs2-types.h:1.2 include/pvfs2-types.h:1.3
> --- include/pvfs2-types.h:1.2 Tue Mar 8 16:37:25 2005
> +++ include/pvfs2-types.h Thu Mar 17 13:40:08 2005
> @@ -157,7 +157,9 @@
> #define PVFS_U_EXECUTE (1 << 6)
> #define PVFS_U_WRITE (1 << 7)
> #define PVFS_U_READ (1 << 8)
> -#define PVFS_G_SGID (1 << 9)
> +/* no PVFS_U_VTX (sticky bit) */
> +#define PVFS_G_SGID (1 << 10)
> +/* no PVFS_U_SGID */
>
> /** Object and attribute types. */
> typedef enum
> Index: src/apps/admin/pvfs2-chmod.c
> diff -u src/apps/admin/pvfs2-chmod.c:1.1 src/apps/admin/pvfs2-chmod.c:1.2
> --- src/apps/admin/pvfs2-chmod.c:1.1 Mon Feb 7 12:57:10 2005
> +++ src/apps/admin/pvfs2-chmod.c Thu Mar 17 13:40:10 2005
> @@ -194,6 +194,7 @@
> int user_perms = 0;
> int group_perms = 0;
> int other_perms = 0;
> + int special_perms = 0;
> int i;
>
> struct options* tmp_opts = NULL;
> @@ -228,19 +229,33 @@
> exit(EXIT_FAILURE);
> }
> }
> - if (strlen(argv[optind]) != 3) {
> + if (strlen(argv[optind]) == 3)
> + {
> + user_perms = check_perm(argv[optind][0]);
> + group_perms = check_perm(argv[optind][1]);
> + other_perms = check_perm(argv[optind][2]);
> + }
> + else if(strlen(argv[optind]) == 4)
> + {
> + special_perms = check_perm(argv[optind][0]);
> + user_perms = check_perm(argv[optind][1]);
> + group_perms = check_perm(argv[optind][2]);
> + other_perms = check_perm(argv[optind][3]);
> + }
> + else
> + {
> usage (argc, argv);
> exit(EXIT_FAILURE);
> }
> - user_perms = check_perm(argv[optind][0]);
> - group_perms = check_perm(argv[optind][1]);
> - other_perms = check_perm(argv[optind][2]);
>
> - if (user_perms == -1 || group_perms == -1 || other_perms == -1) {
> + if (user_perms == -1 || group_perms == -1 || other_perms == -1 ||
> + special_perms == -1)
> + {
> usage(argc,argv);
> exit(EXIT_FAILURE);
> }
> - tmp_opts->perms=(user_perms << 6) | (group_perms << 3) | (other_perms << 0);
> + tmp_opts->perms=(user_perms << 6) | (group_perms << 3) |
> + (other_perms << 0) | (special_perms << 9);
>
> optind = optind + 1;
> tmp_opts->target_count = argc-optind;
> @@ -259,7 +274,8 @@
> static void usage(int argc, char** argv)
> {
> fprintf(stderr,"Usage: %s [-v] mode filename(s)\n",argv[0]);
> - fprintf(stderr," mode - of the form UGO in octal notation\n");
> + fprintf(stderr," mode - of the form UGO or SUGO in octal notation\n");
> + fprintf(stderr," S - the special permissions (setgid, etc.) for the file(s)\n");
> fprintf(stderr," U - the user permissions for the file(s)\n");
> fprintf(stderr," G - the group permissions for the file(s)\n");
> fprintf(stderr," O - the other permissions for the file(s)\n");
> Index: src/client/sysint/sys-create.sm
> diff -u src/client/sysint/sys-create.sm:1.1 src/client/sysint/sys-create.sm:1.2
> --- src/client/sysint/sys-create.sm:1.1 Mon Feb 7 12:57:30 2005
> +++ src/client/sysint/sys-create.sm Thu Mar 17 13:40:13 2005
> @@ -51,6 +57,8 @@
> PINT_client_sm *sm_p, job_status_s *js_p);
> static int create_cleanup(
> PINT_client_sm *sm_p, job_status_s *js_p);
> +static int create_parent_getattr_inspect(
> + PINT_client_sm *sm_p, job_status_s *js_p);
>
> /* completion function prototypes */
> static int create_create_comp_fn(
> @@ -71,6 +79,7 @@
> create_parent_getattr_setup_msgpair,
> create_parent_getattr_xfer_msgpair,
> create_parent_getattr_failure,
> + parent_getattr_inspect,
> dspace_create_setup_msgpair,
> dspace_create_xfer_msgpair,
> dspace_create_failure,
> @@ -103,6 +112,13 @@
> state create_parent_getattr_xfer_msgpair
> {
> jump pvfs2_client_getattr_acache_sm;
> + success => parent_getattr_inspect;
> + default => create_parent_getattr_failure;
> + }
> +
> + state parent_getattr_inspect
> + {
> + run create_parent_getattr_inspect;
> success => dspace_create_setup_msgpair;
> default => create_parent_getattr_failure;
> }
> @@ -991,6 +1011,42 @@
> return 0;
> }
>
> +/** looks at the attributes of the parent directory and decides if it impacts
> + * the file creation in any way
> + */
> +static int create_parent_getattr_inspect(
> + PINT_client_sm *sm_p, job_status_s *js_p)
> +{
> + PVFS_object_attr *attr = NULL;
> +
> + gossip_debug(GOSSIP_CLIENT_DEBUG, "create state: parent_getattr_inspect\n");
> +
> + gossip_debug(GOSSIP_CLIENT_DEBUG, "parent acache_hit: %d\n", sm_p->acache_hit);
> + if(sm_p->acache_hit)
> + {
> + attr = &sm_p->pinode->attr;
> + }
> + else
> + {
> + attr = &sm_p->acache_attr;
> + }
> + assert(attr);
> +
> + gossip_debug(GOSSIP_CLIENT_DEBUG, "parent owner: %d, group: %d, perms: %d\n",
> + (int)attr->owner, (int)attr->group, (int)attr->perms);
> +
> + /* do we have a setgid bit? */
> + if(attr->perms & PVFS_G_SGID)
> + {
> + gossip_debug(GOSSIP_CLIENT_DEBUG, "parent has setgid bit set.\n");
> + gossip_debug(GOSSIP_CLIENT_DEBUG, " - modifying requested attr for new file.\n");
> + sm_p->u.create.sys_attr.group = attr->group;
> + /* note that permission checking is left to server even in this case */
> + }
> +
> + return(1);
> +}
> +
> /*
> * Local variables:
> * mode: c
> ---------------------
> PatchSet 46
> Date: 2005/03/17 21:06:10
> Author: pcarns
> Branch: HEAD
> Tag: (none)
> Log:
> finished setgid support, by making sure that subdirectories are created with
> proper group membership and setgid bit set
> [associate:task2939]
>
> Members:
> src/client/sysint/sys-mkdir.sm:1.1->1.2
>
> Index: src/client/sysint/sys-mkdir.sm
> diff -u src/client/sysint/sys-mkdir.sm:1.1 src/client/sysint/sys-mkdir.sm:1.2
> --- src/client/sysint/sys-mkdir.sm:1.1 Mon Feb 7 12:57:34 2005
> +++ src/client/sysint/sys-mkdir.sm Thu Mar 17 14:06:10 2005
> @@ -41,6 +47,8 @@
> PINT_client_sm *sm_p, job_status_s *js_p);
> static int mkdir_cleanup(
> PINT_client_sm *sm_p, job_status_s *js_p);
> +static int mkdir_parent_getattr_inspect(
> + PINT_client_sm *sm_p, job_status_s *js_p);
>
> static int mkdir_msg_comp_fn(
> void *v_p, struct PVFS_server_resp *resp_p, int index);
> @@ -56,6 +64,7 @@
> mkdir_parent_getattr_setup_msgpair,
> mkdir_parent_getattr_xfer_msgpair,
> mkdir_parent_getattr_failure,
> + parent_getattr_inspect,
> mkdir_msg_setup_msgpair,
> mkdir_msg_xfer_msgpair,
> mkdir_msg_failure,
> @@ -82,6 +91,13 @@
> state mkdir_parent_getattr_xfer_msgpair
> {
> jump pvfs2_client_getattr_acache_sm;
> + success => parent_getattr_inspect;
> + default => mkdir_parent_getattr_failure;
> + }
> +
> + state parent_getattr_inspect
> + {
> + run mkdir_parent_getattr_inspect;
> success => mkdir_msg_setup_msgpair;
> default => mkdir_parent_getattr_failure;
> }
> @@ -540,6 +560,43 @@
> return 0;
> }
>
> +/** looks at the attributes of the parent directory and decides if it impacts
> + * the mkdir in any way
> + */
> +static int mkdir_parent_getattr_inspect(
> + PINT_client_sm *sm_p, job_status_s *js_p)
> +{
> + PVFS_object_attr *attr = NULL;
> +
> + gossip_debug(GOSSIP_CLIENT_DEBUG, "mkdir state: parent_getattr_inspect\n");
> +
> + gossip_debug(GOSSIP_CLIENT_DEBUG, "parent acache_hit: %d\n", sm_p->acache_hit);
> + if(sm_p->acache_hit)
> + {
> + attr = &sm_p->pinode->attr;
> + }
> + else
> + {
> + attr = &sm_p->acache_attr;
> + }
> + assert(attr);
> +
> + gossip_debug(GOSSIP_CLIENT_DEBUG, "parent owner: %d, group: %d, perms: %d\n",
> + (int)attr->owner, (int)attr->group, (int)attr->perms);
> +
> + /* do we have a setgid bit? */
> + if(attr->perms & PVFS_G_SGID)
> + {
> + gossip_debug(GOSSIP_CLIENT_DEBUG, "parent has setgid bit set.\n");
> + gossip_debug(GOSSIP_CLIENT_DEBUG, " - modifying requested attr for new file.\n");
> + sm_p->u.mkdir.sys_attr.group = attr->group;
> + sm_p->u.mkdir.sys_attr.perms |= PVFS_G_SGID;
> + /* note that permission checking is left to server even in this case */
> + }
> +
> + return(1);
> +}
> +
> /*
> * Local variables:
> * mode: c
> ---------------------
> PatchSet 48
> Date: 2005/04/29 20:53:07
> Author: pcarns
> Branch: HEAD
> Tag: (none)
> Log:
> modifications to make sure that setattr returns an error code (EINVAL) if
> it is asked to set permission bits that PVFS2 does not support. This
> currently includes just the setuid and sticky bits.
>
> Members:
> include/pvfs2-types.h:1.3->1.4
> src/client/sysint/sys-setattr.sm:1.2->1.3
> src/kernel/linux-2.6/pvfs2-utils.c:1.4->1.5
>
> Index: include/pvfs2-types.h
> diff -u include/pvfs2-types.h:1.3 include/pvfs2-types.h:1.4
> --- include/pvfs2-types.h:1.3 Thu Mar 17 13:40:08 2005
> +++ include/pvfs2-types.h Fri Apr 29 13:53:07 2005
> @@ -161,6 +161,12 @@
> #define PVFS_G_SGID (1 << 10)
> /* no PVFS_U_SGID */
>
> +/* valid permission mask */
> +#define PVFS_PERM_VALID \
> +(PVFS_O_EXECUTE | PVFS_O_WRITE | PVFS_O_READ | PVFS_G_EXECUTE | \
> + PVFS_G_WRITE | PVFS_G_READ | PVFS_U_EXECUTE | PVFS_U_WRITE | \
> + PVFS_U_READ | PVFS_G_SGID)
> +
> /** Object and attribute types. */
> typedef enum
> {
> Index: src/client/sysint/sys-setattr.sm
> diff -u src/client/sysint/sys-setattr.sm:1.2 src/client/sysint/sys-setattr.sm:1.3
> --- src/client/sysint/sys-setattr.sm:1.2 Thu Mar 17 09:18:17 2005
> +++ src/client/sysint/sys-setattr.sm Fri Apr 29 13:53:11 2005
> @@ -139,6 +139,14 @@
> return ret;
> }
>
> + /* make sure that the permission bits are acceptable */
> + if ((attr.perms & ~PVFS_PERM_VALID) != 0)
> + {
> + gossip_lerr("PVFS_isys_setattr() failure: invalid or unsupported"
> + "permission bits\n");
> + return(-PVFS_EINVAL);
> + }
> +
> sm_p = (PINT_client_sm *)malloc(sizeof(*sm_p));
> if (sm_p == NULL)
> {
> Index: src/kernel/linux-2.6/pvfs2-utils.c
> diff -u src/kernel/linux-2.6/pvfs2-utils.c:1.4 src/kernel/linux-2.6/pvfs2-utils.c:1.5
> --- src/kernel/linux-2.6/pvfs2-utils.c:1.4 Thu Mar 17 09:41:34 2005
> +++ src/kernel/linux-2.6/pvfs2-utils.c Fri Apr 29 13:53:14 2005
> @@ -301,12 +301,24 @@
> if (iattr && (iattr->ia_valid & ATTR_MODE))
> {
> pvfs2_print("[1] converting attr mode %d\n", iattr->ia_mode);
> + if((iattr->ia_mode & (S_ISUID|S_ISVTX)) != 0)
> + {
> + pvfs2_print("User attempted to set setuid or sticky bit; "
> + "returning EINVAL.\n");
> + return(-EINVAL);
> + }
> convert_attribute_mode_to_pvfs_sys_attr(
> iattr->ia_mode, attrs);
> }
> else
> {
> - pvfs2_print("[2] converting attr mode %d\n", inode->i_mode);
> + pvfs2_print("[2] converting attr mode %d\n", inode->i_mode);
> + if((inode->i_mode & (S_ISUID|S_ISVTX)) != 0)
> + {
> + pvfs2_print("User attempted to set setuid or sticky bit; "
> + "returning EINVAL.\n");
> + return(-EINVAL);
> + }
> convert_attribute_mode_to_pvfs_sys_attr(
> inode->i_mode, attrs);
> }
> @@ -444,8 +456,13 @@
> new_op->upcall.req.lookup.parent_refn.fs_id =
> PVFS2_SB(sb)->fs_id;
> }
> - copy_attributes_from_inode(
> + ret = copy_attributes_from_inode(
> inode, &new_op->upcall.req.setattr.attributes, iattr);
> + if(ret < 0)
> + {
> + op_release(new_op);
> + return(ret);
> + }
>
> service_error_exit_op_with_timeout_retry(
> new_op, "pvfs2_inode_setattr", retries, error_exit,
> ---------------------
> PatchSet 86
> Date: 2005/06/13 14:46:21
> Author: pcarns
> Branch: HEAD
> Tag: (none)
> Log:
> add Acxiom copyright notice to changes needed for setgid bit support
>
> Members:
> src/client/sysint/sys-create.sm:1.3->1.4
> src/client/sysint/sys-mkdir.sm:1.2->1.3
>
> Index: src/client/sysint/sys-create.sm
> diff -u src/client/sysint/sys-create.sm:1.3 src/client/sysint/sys-create.sm:1.4
> --- src/client/sysint/sys-create.sm:1.3 Fri Jun 10 11:27:59 2005
> +++ src/client/sysint/sys-create.sm Mon Jun 13 07:46:21 2005
> @@ -1,6 +1,9 @@
> /*
> * (C) 2003 Clemson University and The University of Chicago
> *
> + * Changes by Acxiom Corporation to add setgid support
> + * Copyright © Acxiom Corporation, 2005.
> + *
> * See COPYING in top-level directory.
> */
>
> Index: src/client/sysint/sys-mkdir.sm
> diff -u src/client/sysint/sys-mkdir.sm:1.2 src/client/sysint/sys-mkdir.sm:1.3
> --- src/client/sysint/sys-mkdir.sm:1.2 Thu Mar 17 14:06:10 2005
> +++ src/client/sysint/sys-mkdir.sm Mon Jun 13 07:46:21 2005
> @@ -1,6 +1,9 @@
> /*
> * (C) 2003 Clemson University and The University of Chicago
> *
> + * Changes by Acxiom Corporation to add setgid support
> + * Copyright © Acxiom Corporation, 2005.
> + *
> * See COPYING in top-level directory.
> */
>
>
>
> ------------------------------------------------------------------------
>
> _______________________________________________
> PVFS2-developers mailing list
> PVFS2-developers at beowulf-underground.org
> http://www.beowulf-underground.org/mailman/listinfo/pvfs2-developers
More information about the PVFS2-developers
mailing list