LU-5423 llite: pack suppgid to MDS correctly

The ll_lookup_it() may trigger IT_OPEN RPC to open a file by name.
But at that time, the client does not know the target file's GID,
so it cannot pack the necessary supplementary group ID in the RPC.
Because of missing the supplementary group ID, the RPC maybe fail
for open permission check on the MDS. Under such case, MDS should
return the target file's GID, if the current thread on the client
in the right group (according to the file's GID), the client will
try the IT_OPEN RPC again with the right supplementary group ID.

This patch is also helpful if some other(s) changed the file's GID
after current RPC sent to the MDS with the suppgid as the original
GID by race.

Signed-off-by: Fan Yong <fan.yong@intel.com>
Change-Id: Icaf1ae72b64a27c64c42830d231bae4bca4acb66
Reviewed-on: http://review.whamcloud.com/12476
Tested-by: Jenkins
Tested-by: Maloo <hpdd-maloo@intel.com>
Reviewed-by: Lai Siyao <lai.siyao@intel.com>
Reviewed-by: Andreas Dilger <andreas.dilger@intel.com>
Reviewed-by: Oleg Drokin <oleg.drokin@intel.com>

diff --git a/lustre/include/lustre/lustre_idl.h b/lustre/include/lustre/lustre_idl.h
index 25f0b3b..7e82994 100644 (file)
--- a/lustre/include/lustre/lustre_idl.h
+++ b/lustre/include/lustre/lustre_idl.h
@@ -2194,6 +2194,7 @@ extern void lustre_swab_generic_32s (__u32 *val);
 #define DISP_OPEN_LOCK       0x02000000
 #define DISP_OPEN_LEASE      0x04000000
 #define DISP_OPEN_STRIPE     0x08000000
 #define DISP_OPEN_LOCK       0x02000000
 #define DISP_OPEN_LEASE      0x04000000
 #define DISP_OPEN_STRIPE     0x08000000

+#define DISP_OPEN_DENY      0x10000000

 
 /* INODE LOCK PARTS */
 #define MDS_INODELOCK_LOOKUP 0x000001  /* For namespace, dentry etc, and also
 
 /* INODE LOCK PARTS */
 #define MDS_INODELOCK_LOOKUP 0x000001  /* For namespace, dentry etc, and also

diff --git a/lustre/llite/namei.c b/lustre/llite/namei.c
index 28002fa..97ea1f1 100644 (file)
--- a/lustre/llite/namei.c
+++ b/lustre/llite/namei.c
@@ -515,10 +515,10 @@ static int ll_lookup_it_finish(struct ptlrpc_request *request,
 static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry,
                                   struct lookup_intent *it)
 {
 static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry,
                                   struct lookup_intent *it)
 {

-        struct lookup_intent lookup_it = { .it_op = IT_LOOKUP };
-        struct dentry *save = dentry, *retval;
-        struct ptlrpc_request *req = NULL;
-        struct md_op_data *op_data;
+       struct lookup_intent lookup_it = { .it_op = IT_LOOKUP };
+       struct dentry *save = dentry, *retval;
+       struct ptlrpc_request *req = NULL;
+       struct md_op_data *op_data = NULL;

         __u32 opc;
         int rc;
         ENTRY;
         __u32 opc;
         int rc;
         ENTRY;


@@ -549,18 +549,43 @@ static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry,
 
        op_data = ll_prep_md_op_data(NULL, parent, NULL, dentry->d_name.name,
                                     dentry->d_name.len, 0, opc, NULL);
 
        op_data = ll_prep_md_op_data(NULL, parent, NULL, dentry->d_name.name,
                                     dentry->d_name.len, 0, opc, NULL);

-        if (IS_ERR(op_data))
-                RETURN((void *)op_data);
+       if (IS_ERR(op_data))
+               RETURN((void *)op_data);

 
 

-        /* enforce umask if acl disabled or MDS doesn't support umask */
-        if (!IS_POSIXACL(parent) || !exp_connect_umask(ll_i2mdexp(parent)))
+       /* enforce umask if acl disabled or MDS doesn't support umask */
+       if (!IS_POSIXACL(parent) || !exp_connect_umask(ll_i2mdexp(parent)))

                it->it_create_mode &= ~current_umask();
 
        rc = md_intent_lock(ll_i2mdexp(parent), op_data, it, &req,
                            &ll_md_blocking_ast, 0);
                it->it_create_mode &= ~current_umask();
 
        rc = md_intent_lock(ll_i2mdexp(parent), op_data, it, &req,
                            &ll_md_blocking_ast, 0);

-        ll_finish_md_op_data(op_data);
-        if (rc < 0)
-                GOTO(out, retval = ERR_PTR(rc));
+       /* If the MDS allows the client to chgrp (CFS_SETGRP_PERM), but the
+        * client does not know which suppgid should be sent to the MDS, or
+        * some other(s) changed the target file's GID after this RPC sent
+        * to the MDS with the suppgid as the original GID, then we should
+        * try again with right suppgid. */
+       if (rc == -EACCES && it->it_op & IT_OPEN &&
+           it_disposition(it, DISP_OPEN_DENY)) {
+               struct mdt_body *body;
+
+               LASSERT(req != NULL);
+
+               body = req_capsule_server_get(&req->rq_pill, &RMF_MDT_BODY);
+               if (op_data->op_suppgids[0] == body->mbo_gid ||
+                   op_data->op_suppgids[1] == body->mbo_gid ||
+                   !in_group_p(make_kgid(&init_user_ns, body->mbo_gid)))
+                       GOTO(out, retval = ERR_PTR(-EACCES));
+
+               fid_zero(&op_data->op_fid2);
+               op_data->op_suppgids[1] = body->mbo_gid;
+               ptlrpc_req_finished(req);
+               req = NULL;
+               ll_intent_release(it);
+               rc = md_intent_lock(ll_i2mdexp(parent), op_data, it, &req,
+                                   &ll_md_blocking_ast, 0);
+       }
+
+       if (rc < 0)
+               GOTO(out, retval = ERR_PTR(rc));

 
        rc = ll_lookup_it_finish(req, it, parent, &dentry);
         if (rc != 0) {
 
        rc = ll_lookup_it_finish(req, it, parent, &dentry);
         if (rc != 0) {


@@ -575,10 +600,12 @@ static struct dentry *ll_lookup_it(struct inode *parent, struct dentry *dentry,
         }
         ll_lookup_finish_locks(it, dentry);
 
         }
         ll_lookup_finish_locks(it, dentry);
 

-       retval = (dentry == save) ? NULL : dentry;
-       EXIT;
+       GOTO(out, retval = (dentry == save) ? NULL : dentry);

 
 out:
 
 out:

+       if (op_data != NULL && !IS_ERR(op_data))
+               ll_finish_md_op_data(op_data);
+

        ptlrpc_req_finished(req);
        return retval;
 }
        ptlrpc_req_finished(req);
        return retval;
 }

diff --git a/lustre/mdt/mdt_internal.h b/lustre/mdt/mdt_internal.h
index ea2f66d..78b09bf 100644 (file)
--- a/lustre/mdt/mdt_internal.h
+++ b/lustre/mdt/mdt_internal.h
@@ -766,6 +766,7 @@ const struct lu_buf *mdt_buf_const(const struct lu_env *env,
 void mdt_dump_lmm(int level, const struct lov_mds_md *lmm, __u64 valid);
 void mdt_dump_lmv(unsigned int level, const union lmv_mds_md *lmv);
 
 void mdt_dump_lmm(int level, const struct lov_mds_md *lmm, __u64 valid);
 void mdt_dump_lmv(unsigned int level, const union lmv_mds_md *lmv);
 

+bool allow_client_chgrp(struct mdt_thread_info *info, struct lu_ucred *uc);

 int mdt_check_ucred(struct mdt_thread_info *);
 int mdt_init_ucred(struct mdt_thread_info *, struct mdt_body *);
 int mdt_init_ucred_reint(struct mdt_thread_info *);
 int mdt_check_ucred(struct mdt_thread_info *);
 int mdt_init_ucred(struct mdt_thread_info *, struct mdt_body *);
 int mdt_init_ucred_reint(struct mdt_thread_info *);

diff --git a/lustre/mdt/mdt_lib.c b/lustre/mdt/mdt_lib.c
index 1033f58..ce695db 100644 (file)
--- a/lustre/mdt/mdt_lib.c
+++ b/lustre/mdt/mdt_lib.c
@@ -318,6 +318,43 @@ out:
        return rc;
 }
 
        return rc;
 }
 

+/**
+ * Check whether allow the client to set supplementary group IDs or not.
+ *
+ * \param[in] info     pointer to the thread context
+ * \param[in] uc       pointer to the RPC user descriptor
+ *
+ * \retval             true if allow to set supplementary group IDs
+ * \retval             false for other cases
+ */
+bool allow_client_chgrp(struct mdt_thread_info *info, struct lu_ucred *uc)
+{
+       __u32 remote = exp_connect_rmtclient(info->mti_exp);
+       __u32 perm;
+
+       /* 1. If identity_upcall is disabled, then forbid remote client to set
+        *    supplementary group IDs, but permit local client to do that. */
+       if (is_identity_get_disabled(info->mti_mdt->mdt_identity_cache)) {
+               if (remote)
+                       return false;
+
+               return true;
+       }
+
+       /* 2. If fail to get related identities, then forbid any client to
+        *    set supplementary group IDs. */
+       if (uc->uc_identity == NULL)
+               return false;
+
+       /* 3. Check the permission in the identities. */
+       perm = mdt_identity_get_perm(uc->uc_identity, remote,
+                                    mdt_info_req(info)->rq_peer.nid);
+       if (perm & CFS_SETGRP_PERM)
+               return true;
+
+       return false;
+}
+

 int mdt_check_ucred(struct mdt_thread_info *info)
 {
         struct ptlrpc_request   *req = mdt_info_req(info);
 int mdt_check_ucred(struct mdt_thread_info *info)
 {
         struct ptlrpc_request   *req = mdt_info_req(info);

diff --git a/lustre/mdt/mdt_open.c b/lustre/mdt/mdt_open.c
index c779056..97918c4 100644 (file)
--- a/lustre/mdt/mdt_open.c
+++ b/lustre/mdt/mdt_open.c
@@ -755,8 +755,18 @@ static int mdt_mfd_open(struct mdt_thread_info *info, struct mdt_object *p,
 
         rc = mo_open(info->mti_env, mdt_object_child(o),
                      created ? flags | MDS_OPEN_CREATED : flags);
 
         rc = mo_open(info->mti_env, mdt_object_child(o),
                      created ? flags | MDS_OPEN_CREATED : flags);

-        if (rc)
-                GOTO(err_out, rc);
+       if (rc != 0) {
+               /* If we allow the client to chgrp (CFS_SETGRP_PERM), but the
+                * client does not know which suppgid should be sent to the MDS,
+                * or some other(s) changed the target file's GID after this RPC
+                * sent to the MDS with the suppgid as the original GID, then we
+                * give the client another chance to send the right suppgid. */
+               if (rc == -EACCES &&
+                   allow_client_chgrp(info, lu_ucred(info->mti_env)))
+                       mdt_set_disposition(info, rep, DISP_OPEN_DENY);
+
+               GOTO(err_out, rc);
+       }

 
        mfd = mdt_mfd_new(med);
        if (mfd == NULL)
 
        mfd = mdt_mfd_new(med);
        if (mfd == NULL)

diff --git a/lustre/tests/sanity-sec.sh b/lustre/tests/sanity-sec.sh
index a70ea40..1f5ae14 100644 (file)
--- a/lustre/tests/sanity-sec.sh
+++ b/lustre/tests/sanity-sec.sh
@@ -7,8 +7,8 @@
 set -e
 
 ONLY=${ONLY:-"$*"}
 set -e
 
 ONLY=${ONLY:-"$*"}

-# bug number for skipped test: 19430 LU-5423 19967 19967
-ALWAYS_EXCEPT="                2     4       5     6    $SANITY_SEC_EXCEPT"
+# bug number for skipped test: 19430 19967 19967
+ALWAYS_EXCEPT="                2     5     6    $SANITY_SEC_EXCEPT"

 # UPDATE THE COMMENT ABOVE WITH BUG NUMBERS WHEN CHANGING ALWAYS_EXCEPT!
 
 [ "$ALWAYS_EXCEPT$EXCEPT" ] && \
 # UPDATE THE COMMENT ABOVE WITH BUG NUMBERS WHEN CHANGING ALWAYS_EXCEPT!
 
 [ "$ALWAYS_EXCEPT$EXCEPT" ] && \