diff options
author | Atin Mukherjee <amukherj@redhat.com> | 2016-02-11 15:37:08 +0530 |
---|---|---|
committer | Kaushal M <kaushal@redhat.com> | 2016-08-15 23:27:23 -0700 |
commit | 42b5b9bc63891a3447db56224713c5b1028549c5 (patch) | |
tree | 5d7b2ccf595a9add3d7438d05cc0124f36deca30 /rpc | |
parent | cfba6098a4ef8c672969d69678fcdca7c8bb6399 (diff) |
rpc : build_prog_details should iterate program list inside critical section
Backport of http://review.gluster.org/13428
While I was analyzing a glusterd crash from free_prog_details, a code
walkthrough detects that we iterate over the rpc svc program list without been
inside the criticial section. This opens up a possibility of a crash when there
is a concurrent writer updating the same list. Solution is to read the list
inside lock.
> Reviewed-on: http://review.gluster.org/13428
> Smoke: Gluster Build System <jenkins@build.gluster.com>
> CentOS-regression: Gluster Build System <jenkins@build.gluster.com>
> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
> Reviewed-by: Niels de Vos <ndevos@redhat.com>
> Reviewed-by: Raghavendra G <rgowdapp@redhat.com>
BUG: 1310970
Change-Id: Ib4b4b0022a9535e139cd3c00574aab23f07aa9d2
Signed-off-by: Atin Mukherjee <amukherj@redhat.com>
Signed-off-by: Oleksandr Natalenko <oleksandr@natalenko.name>
Reviewed-on: http://review.gluster.org/13488
Smoke: Gluster Build System <jenkins@build.gluster.org>
NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org>
CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
Reviewed-by: Kaushal M <kaushal@redhat.com>
Diffstat (limited to 'rpc')
-rw-r--r-- | rpc/rpc-lib/src/rpcsvc.c | 33 |
1 files changed, 20 insertions, 13 deletions
diff --git a/rpc/rpc-lib/src/rpcsvc.c b/rpc/rpc-lib/src/rpcsvc.c index b0eea194347..7d5cc937d86 100644 --- a/rpc/rpc-lib/src/rpcsvc.c +++ b/rpc/rpc-lib/src/rpcsvc.c @@ -1889,21 +1889,28 @@ build_prog_details (rpcsvc_request_t *req, gf_dump_rsp *rsp) if (!req || !req->trans || !req->svc) goto out; - list_for_each_entry (program, &req->svc->programs, program) { - prog = GF_CALLOC (1, sizeof (*prog), 0); - if (!prog) - goto out; - prog->progname = program->progname; - prog->prognum = program->prognum; - prog->progver = program->progver; - if (!rsp->prog) - rsp->prog = prog; + pthread_mutex_lock (&req->svc->rpclock); + { + list_for_each_entry (program, &req->svc->programs, program) { + prog = GF_CALLOC (1, sizeof (*prog), 0); + if (!prog) + goto unlock; + + prog->progname = program->progname; + prog->prognum = program->prognum; + prog->progver = program->progver; + + if (!rsp->prog) + rsp->prog = prog; + if (prev) + prev->next = prog; + prev = prog; + } if (prev) - prev->next = prog; - prev = prog; + ret = 0; } - if (prev) - ret = 0; +unlock: + pthread_mutex_unlock (&req->svc->rpclock); out: return ret; } |