(1) Drop unnecessary permission check for name_{insert,remove}.

(2) Do local permission check for name_{insert,remove} before remote ops.
(3) Add some comment.

diff --git a/lustre/cmm/cmm_object.c b/lustre/cmm/cmm_object.c
index 1877680..ab6f552 100644 (file)
--- a/lustre/cmm/cmm_object.c
+++ b/lustre/cmm/cmm_object.c
@@ -548,6 +548,11 @@ static int cml_rename(const struct lu_env *env, struct md_object *mo_po,
 
         if (mo_t && lu_object_exists(&mo_t->mo_lu) < 0) {
                 /* mo_t is remote object and there is RPC to unlink it */
+                /*
+                 * XXX: before remote unlink, maybe need local sanity check
+                 * for mdo_rename first, or do some revocation for remote
+                 * unlink if mdo_rename failed.
+                 */
                 rc = mo_ref_del(env, md_object_next(mo_t), ma);
                 if (rc)
                         RETURN(rc);
@@ -887,6 +892,11 @@ static int cmr_create(const struct lu_env *env, struct md_object *mo_p,
         }
 #endif
 
+        /* Local permission check for name_insert before remote ops. */
+        rc = mo_permission(env, md_object_next(mo_p), MAY_WRITE);
+        if (rc)
+                RETURN(rc);
+
         /* Remote object creation and local name insert. */
         rc = mo_object_create(env, md_object_next(mo_c), spec, ma);
         if (rc == 0) {
@@ -911,6 +921,11 @@ static int cmr_link(const struct lu_env *env, struct md_object *mo_p,
         if (rc == 0) {
                 rc = -EEXIST;
         } else if (rc == -ENOENT) {
+                /* Local permission check for name_insert before remote ops. */
+                rc = mo_permission(env, md_object_next(mo_p), MAY_WRITE);
+                if (rc)
+                        RETURN(rc);
+
                 rc = mo_ref_add(env, md_object_next(mo_s));
                 if (rc == 0) {
                         rc = mdo_name_insert(env, md_object_next(mo_p), name,
@@ -927,6 +942,11 @@ static int cmr_unlink(const struct lu_env *env, struct md_object *mo_p,
         int rc;
         ENTRY;
 
+        /* Local permission check for name_remove before remote ops. */
+        rc = mo_permission(env, md_object_next(mo_p), MAY_WRITE);
+        if (rc)
+                RETURN(rc);
+
         rc = mo_ref_del(env, md_object_next(mo_c), ma);
         if (rc == 0) {
                 rc = mdo_name_remove(env, md_object_next(mo_p), name,
@@ -951,6 +971,12 @@ static int cmr_rename(const struct lu_env *env,
                 RETURN(rc);
 
         LASSERT(mo_t == NULL);
+
+        /* Local permission check for name_remove before remote ops. */
+        rc = mo_permission(env, md_object_next(mo_po), MAY_WRITE);
+        if (rc)
+                RETURN(rc);
+
         /* the mo_pn is remote directory, so we cannot even know if there is
          * mo_t or not. Therefore mo_t is NULL here but remote server should do
          * lookup and process this further */
@@ -974,6 +1000,11 @@ static int cmr_rename_tgt(const struct lu_env *env,
         int rc;
         ENTRY;
         /* target object is remote one */
+        /*
+         * XXX: before remote unlink, maybe need local sanity check
+         * for mdo_rename_tgt first, or do some revocation for remote
+         * unlink if mdo_rename_tgt failed.
+         */
         rc = mo_ref_del(env, md_object_next(mo_t), ma);
         /* continue locally with name handling only */
         if (rc == 0)

diff --git a/lustre/cmm/cmm_split.c b/lustre/cmm/cmm_split.c
index 9db11d5..8951f58 100644 (file)
--- a/lustre/cmm/cmm_split.c
+++ b/lustre/cmm/cmm_split.c
@@ -408,6 +408,7 @@ static int cmm_split_remove_entry(const struct lu_env *env,
                 GOTO(cleanup, rc = -ENOMEM);
 
         memcpy(name, ent->lde_name, le16_to_cpu(ent->lde_namelen));
+        /* No permission check for name_remove when split */
         rc = mdo_name_remove(env, md_object_next(mo),
                              name, is_dir);
         OBD_FREE(name, le16_to_cpu(ent->lde_namelen) + 1);

diff --git a/lustre/mdd/mdd_dir.c b/lustre/mdd/mdd_dir.c
index 280afc9..d16d29e 100644 (file)
--- a/lustre/mdd/mdd_dir.c
+++ b/lustre/mdd/mdd_dir.c
@@ -623,6 +623,7 @@ out_trans:
         return rc;
 }
 
+#if 0
 static int mdd_ni_sanity_check(const struct lu_env *env,
                                struct md_object *pobj,
                                const char *name,
@@ -639,6 +640,7 @@ static int mdd_ni_sanity_check(const struct lu_env *env,
         RETURN(mdd_permission_internal_locked(env, obj, NULL,
                                               MAY_WRITE | MAY_EXEC));
 }
+#endif
 
 /*
  * Partial operation.
@@ -663,17 +665,24 @@ static int mdd_name_insert(const struct lu_env *env, struct md_object *pobj,
         dlh = mdd_pdo_write_lock(env, mdd_obj, name);
         if (dlh == NULL)
                 GOTO(out_trans, rc = -ENOMEM);
+#if 0
+        /*
+         * For some case, no need permission check, e.g. split_dir.
+         * When need permission check, do it before name_insert.
+         */
         rc = mdd_ni_sanity_check(env, pobj, name, fid);
         if (rc)
                 GOTO(out_unlock, rc);
+#endif
 
         rc = __mdd_index_insert(env, mdd_obj, fid, name, is_dir,
                                 handle, BYPASS_CAPA);
-        if (rc == 0) {
-                la->la_ctime = la->la_atime = CURRENT_SECONDS;
-                la->la_valid = LA_ATIME | LA_CTIME;
-                rc = mdd_attr_set_internal_locked(env, mdd_obj, la, handle, 0);
-        }
+        if (rc)
+                GOTO(out_unlock, rc);
+
+        la->la_ctime = la->la_atime = CURRENT_SECONDS;
+        la->la_valid = LA_ATIME | LA_CTIME;
+        rc = mdd_attr_set_internal_locked(env, mdd_obj, la, handle, 0);
         EXIT;
 out_unlock:
         mdd_pdo_write_unlock(env, mdd_obj, dlh);
@@ -682,6 +691,7 @@ out_trans:
         return rc;
 }
 
+#if 0
 static int mdd_nr_sanity_check(const struct lu_env *env,
                                struct md_object *pobj,
                                const char *name)
@@ -699,6 +709,7 @@ static int mdd_nr_sanity_check(const struct lu_env *env,
         RETURN(mdd_permission_internal_locked(env, obj, NULL,
                                               MAY_WRITE | MAY_EXEC));
 }
+#endif
 
 /*
  * Partial operation.
@@ -723,9 +734,15 @@ static int mdd_name_remove(const struct lu_env *env,
         dlh = mdd_pdo_write_lock(env, mdd_obj, name);
         if (dlh == NULL)
                 GOTO(out_trans, rc = -ENOMEM);
+#if 0
+        /*
+         * For some case, no need permission check, e.g. split_dir.
+         * When need permission check, do it before name_remove.
+         */
         rc = mdd_nr_sanity_check(env, pobj, name);
         if (rc)
                 GOTO(out_unlock, rc);
+#endif
 
         rc = __mdd_index_delete(env, mdd_obj, name, is_dir,
                                 handle, BYPASS_CAPA);

diff --git a/lustre/mdt/mdt_handler.c b/lustre/mdt/mdt_handler.c
index 5e239f9..737877a 100644 (file)
--- a/lustre/mdt/mdt_handler.c
+++ b/lustre/mdt/mdt_handler.c
@@ -1117,6 +1117,7 @@ static int mdt_write_dir_page(struct mdt_thread_info *info, struct page *page,
                         GOTO(out, rc = -ENOMEM);
 
                 memcpy(name, ent->lde_name, le16_to_cpu(ent->lde_namelen));
+                /* No permission check for name_insert when write_dir_page */
                 rc = mdo_name_insert(info->mti_env,
                                      md_object_next(&object->mot_obj),
                                      name, lf, is_dir);

diff --git a/lustre/mdt/mdt_reint.c b/lustre/mdt/mdt_reint.c
index f35d2b8..f5b08fc 100644 (file)
--- a/lustre/mdt/mdt_reint.c
+++ b/lustre/mdt/mdt_reint.c
@@ -613,6 +613,12 @@ static int mdt_reint_rename_tgt(struct mdt_thread_info *info)
                                     mdt_object_child(mtgt), rr->rr_fid2,
                                     rr->rr_tgt, ma);
         } else /* -ENOENT */ {
+                /* Do permission check for name_insert first */
+                rc = mo_permission(info->mti_env, mdt_object_child(mtgtdir),
+                                   MAY_WRITE);
+                if (rc)
+                        GOTO(out_unlock_tgtdir, rc);
+
                 rc = mdo_name_insert(info->mti_env, mdt_object_child(mtgtdir),
                                      rr->rr_tgt, rr->rr_fid2,
                                      S_ISDIR(ma->ma_attr.la_mode));