summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCihangir Akturk <cakturk@gmail.com>2017-08-07 13:43:00 +0300
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2017-08-18 17:16:51 -0700
commit0de79ffc09a2d28640b4b3d9129080da5646ef25 (patch)
tree476488cb5b74d7618ca5f09337744cbb8a3e8a88
parentbab87505c3284ac3345463abb8af9e6774e7e9f8 (diff)
staging: lustre: mgc: fix potential use after free in error path
The config_log_add() function first calls config_log_put() with the variable 'cld' and then jumps to label 'out_cld', which will call the same function with the same 'cld' variable. However, at this point, 'cld' might have been already freed by the first invocation of config_log_put(). Even if we remove the invocation at that point, we will still get into trouble. This is because, in the error path, just below the label 'out_cld', we try to put 'params_cls' and 'sptlrpc_cld', which might also have been freed by config_log_put(). The point is that, config_llog_data::cld_sptlrpc and config_llog_data::cld_params members are assigned at the beginning of this function. To avoid this, do not call config_log_put() inside the else block, immediately jump to 'out_cld' instead. Moreover, remove assignments to config_llog_data::cld_sptlrpc and config_llog_data::cld_params at the beginning, since we already assign them below in the function with 'cld_lock' held. As an additional benefit, code size gets smaller. before: text data bss dec hex filename 26188 2256 4208 32652 7f8c drivers/staging/lustre/lustre/mgc/mgc_request.o after: text data bss dec hex filename 26092 2256 4208 32556 7f2c drivers/staging/lustre/lustre/mgc/mgc_request.o Signed-off-by: Cihangir Akturk <cakturk@gmail.com> Reviewed-by: James Simmons <jsimmons@infradead.org> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
-rw-r--r--drivers/staging/lustre/lustre/mgc/mgc_request.c16
1 files changed, 3 insertions, 13 deletions
diff --git a/drivers/staging/lustre/lustre/mgc/mgc_request.c b/drivers/staging/lustre/lustre/mgc/mgc_request.c
index eee0b667a33c..36c304953b2d 100644
--- a/drivers/staging/lustre/lustre/mgc/mgc_request.c
+++ b/drivers/staging/lustre/lustre/mgc/mgc_request.c
@@ -288,7 +288,7 @@ config_log_add(struct obd_device *obd, char *logname,
struct config_llog_data *cld;
struct config_llog_data *sptlrpc_cld;
struct config_llog_data *params_cld;
- bool locked = false;
+ struct config_llog_data *recover_cld = NULL;
char seclogname[32];
char *ptr;
int rc;
@@ -333,20 +333,14 @@ config_log_add(struct obd_device *obd, char *logname,
goto out_params;
}
- cld->cld_sptlrpc = sptlrpc_cld;
- cld->cld_params = params_cld;
-
LASSERT(lsi->lsi_lmd);
if (!(lsi->lsi_lmd->lmd_flags & LMD_FLG_NOIR)) {
- struct config_llog_data *recover_cld;
-
ptr = strrchr(seclogname, '-');
if (ptr) {
*ptr = 0;
} else {
CERROR("%s: sptlrpc log name not correct, %s: rc = %d\n",
obd->obd_name, seclogname, -EINVAL);
- config_log_put(cld);
rc = -EINVAL;
goto out_cld;
}
@@ -355,14 +349,10 @@ config_log_add(struct obd_device *obd, char *logname,
rc = PTR_ERR(recover_cld);
goto out_cld;
}
-
- mutex_lock(&cld->cld_lock);
- locked = true;
- cld->cld_recover = recover_cld;
}
- if (!locked)
- mutex_lock(&cld->cld_lock);
+ mutex_lock(&cld->cld_lock);
+ cld->cld_recover = recover_cld;
cld->cld_params = params_cld;
cld->cld_sptlrpc = sptlrpc_cld;
mutex_unlock(&cld->cld_lock);