From 2fef7fd122f3f97bbf12339da70c4025bceb336e Mon Sep 17 00:00:00 2001 From: "hongchao.zhang" Date: Fri, 19 Aug 2011 17:00:18 +0800 Subject: [PATCH] LU-604 open non-exist object should return ENOENT 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 Reviewed-on: http://review.whamcloud.com/1271 Tested-by: Hudson Reviewed-by: Fan Yong Reviewed-by: Andreas Dilger Tested-by: Maloo --- lustre/llite/file.c | 56 +++++++++++++++++++++++++++-------------------------- 1 file changed, 29 insertions(+), 27 deletions(-) diff --git a/lustre/llite/file.c b/lustre/llite/file.c index 53c9f43..3b2cf2e 100644 --- a/lustre/llite/file.c +++ b/lustre/llite/file.c @@ -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); } -- 1.8.3.1