Whamcloud - gitweb
LU-604 open non-exist object should return ENOENT
authorhongchao.zhang <hongchao.zhang@whamcloud.com>
Fri, 19 Aug 2011 09:00:18 +0000 (17:00 +0800)
committerJohann Lombardi <johann@whamcloud.com>
Mon, 12 Dec 2011 23:13:49 +0000 (18:13 -0500)
when call ll_intent_file_open() against non-exist object, we cannot obtain
RPC request refcount after the call, so subsequent operation should not use
such internal RPC request to avoid to access freed memory.

Change-Id: Ic8cd2a7798d2d454cbe5931eb93b92b1a44cca4b
Signed-off-by: Hongchao Zhang<hongchao.zhang@whamcloud.com>
Reviewed-on: http://review.whamcloud.com/1271
Tested-by: Hudson
Reviewed-by: Fan Yong <yong.fan@whamcloud.com>
Reviewed-by: Andreas Dilger <adilger@whamcloud.com>
Tested-by: Maloo <whamcloud.maloo@gmail.com>
lustre/llite/file.c

index 53c9f43..3b2cf2e 100644 (file)
@@ -328,6 +328,9 @@ static int ll_intent_file_open(struct file *file, void *lmm,
                 GOTO(out, rc);
         }
 
+        if (it_disposition(itp, DISP_LOOKUP_NEG))
+                GOTO(out, rc = -ENOENT);
+
         if (rc != 0 || it_open_error(DISP_OPEN_OPEN, itp)) {
                 rc = rc ? rc : it_open_error(DISP_OPEN_OPEN, itp);
                 CDEBUG(D_VFSTRACE, "lock enqueue: err: %d\n", rc);
@@ -408,9 +411,8 @@ int ll_file_open(struct inode *inode, struct file *file)
         struct lookup_intent *it, oit = { .it_op = IT_OPEN,
                                           .it_flags = file->f_flags };
         struct lov_stripe_md *lsm;
-        struct ptlrpc_request *req = NULL;
-        struct obd_client_handle **och_p;
-        __u64 *och_usecount;
+        struct obd_client_handle **och_p = NULL;
+        __u64 *och_usecount = NULL;
         struct ll_file_data *fd;
         int rc = 0, opendir_set = 0;
         ENTRY;
@@ -427,7 +429,7 @@ int ll_file_open(struct inode *inode, struct file *file)
 
         fd = ll_file_data_get();
         if (fd == NULL)
-                RETURN(-ENOMEM);
+                GOTO(out_och_free, rc = -ENOMEM);
 
         if (S_ISDIR(inode->i_mode)) {
                 spin_lock(&lli->lli_lock);
@@ -502,7 +504,6 @@ restart:
                         rc = it_open_error(DISP_OPEN_OPEN, it);
                         if (rc) {
                                 up(&lli->lli_och_sem);
-                                ll_file_data_put(fd);
                                 GOTO(out_openerr, rc);
                         }
                         ll_release_openhandle(file->f_dentry, it);
@@ -526,20 +527,17 @@ restart:
                         it->it_create_mode |= M_CHECK_STALE;
                         rc = ll_intent_file_open(file, NULL, 0, it);
                         it->it_create_mode &= ~M_CHECK_STALE;
-                        if (rc) {
-                                ll_file_data_put(fd);
+                        if (rc)
                                 GOTO(out_openerr, rc);
-                        }
+
                         goto restart;
                 }
 
                 OBD_ALLOC(*och_p, sizeof (struct obd_client_handle));
-                if (!*och_p) {
-                        ll_file_data_put(fd);
+                if (!*och_p)
                         GOTO(out_och_free, rc = -ENOMEM);
-                }
+
                 (*och_usecount)++;
-               req = it->d.lustre.it_data;
 
                 /* mdc_intent_lock() didn't get a request ref if there was an
                  * open error, so don't do cleanup on the request here
@@ -547,42 +545,45 @@ restart:
                 /* XXX (green): Should not we bail out on any error here, not
                  * just open error? */
                 rc = it_open_error(DISP_OPEN_OPEN, it);
-                if (rc) {
-                        ll_file_data_put(fd);
+                if (rc)
                         GOTO(out_och_free, rc);
-                }
+
+                LASSERT(it_disposition(it, DISP_ENQ_OPEN_REF));
 
                 ll_stats_ops_tally(ll_i2sbi(inode), LPROC_LL_OPEN, 1);
                 rc = ll_local_open(file, it, fd, *och_p);
                 LASSERTF(rc == 0, "rc = %d\n", rc);
         }
         up(&lli->lli_och_sem);
+        fd = NULL;
 
         /* Must do this outside lli_och_sem lock to prevent deadlock where
            different kind of OPEN lock for this same inode gets cancelled
            by ldlm_cancel_lru */
         if (!S_ISREG(inode->i_mode))
-                GOTO(out, rc);
+                GOTO(out_och_free, rc);
 
         lsm = lli->lli_smd;
         if (lsm == NULL) {
                 if (file->f_flags & O_LOV_DELAY_CREATE ||
                     !(file->f_mode & FMODE_WRITE)) {
                         CDEBUG(D_INODE, "object creation was delayed\n");
-                        GOTO(out, rc);
+                        GOTO(out_och_free, rc);
                 }
         }
         file->f_flags &= ~O_LOV_DELAY_CREATE;
-        GOTO(out, rc);
- out:
-        ptlrpc_req_finished(req);
-        if (req)
+        GOTO(out_och_free, rc);
+
+out_och_free:
+        if (it && it_disposition(it, DISP_ENQ_OPEN_REF)) {
+                ptlrpc_req_finished(it->d.lustre.it_data);
                 it_clear_disposition(it, DISP_ENQ_OPEN_REF);
+        }
+
         if (rc == 0) {
                 ll_open_complete(inode);
         } else {
-out_och_free:
-                if (*och_p) {
+                if (och_p && *och_p) {
                         OBD_FREE(*och_p, sizeof (struct obd_client_handle));
                         *och_p = NULL; /* OBD_FREE writes some magic there */
                         (*och_usecount)--;
@@ -591,6 +592,8 @@ out_och_free:
 out_openerr:
                 if (opendir_set != 0)
                         ll_stop_statahead(inode, lli->lli_opendir_key);
+                if (fd != NULL)
+                        ll_file_data_put(fd);
         }
 
         return rc;
@@ -2304,8 +2307,6 @@ int ll_lov_setstripe_ea_info(struct inode *inode, struct file *file,
         rc = ll_intent_file_open(file, lum, lum_size, &oit);
         if (rc)
                 GOTO(out, rc);
-        if (it_disposition(&oit, DISP_LOOKUP_NEG))
-                GOTO(out_req_free, rc = -ENOENT);
         rc = oit.d.lustre.it_status;
         if (rc < 0)
                 GOTO(out_req_free, rc);
@@ -2796,9 +2797,10 @@ int ll_release_openhandle(struct dentry *dentry, struct lookup_intent *it)
         OBD_FREE(och, sizeof(*och));
  out:
         /* this one is in place of ll_file_open */
-        if (it_disposition(it, DISP_ENQ_OPEN_REF))
+        if (it_disposition(it, DISP_ENQ_OPEN_REF)) {
                 ptlrpc_req_finished(it->d.lustre.it_data);
-        it_clear_disposition(it, DISP_ENQ_OPEN_REF);
+                it_clear_disposition(it, DISP_ENQ_OPEN_REF);
+        }
         RETURN(rc);
 }