summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorN Balachandran <nbalacha@redhat.com>2017-05-29 15:21:39 +0530
committerShyamsundar Ranganathan <srangana@redhat.com>2017-06-06 12:54:48 +0000
commita443560b541bfe854291229ee9407498f60a8e97 (patch)
tree32d829f12c206d1a6d45d88d3baa6f7a10d0848d
parent42f325c3284482899e8d9f72f9beb8cf00294d43 (diff)
perf/ioc: Fix race causing crash when accessing freed page
ioc_inode_wakeup does not lock the ioc_inode for the duration of the operation, leaving a window where ioc_prune could find a NULL waitq and hence free the page which ioc_inode_wakeup later tries to access. Thanks to Mohit for the analysis. credit: moagrawa@redhat.com > BUG: 1456385 > Signed-off-by: N Balachandran <nbalacha@redhat.com> > Reviewed-on: https://review.gluster.org/17410 > Reviewed-by: Raghavendra G <rgowdapp@redhat.com> > Tested-by: Raghavendra G <rgowdapp@redhat.com> > 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: Jeff Darcy <jeff@pl.atyp.us> Change-Id: I54b064857e2694826d0c03b23f8014e3984a3330 BUG: 1457058 Signed-off-by: N Balachandran <nbalacha@redhat.com> Reviewed-on: https://review.gluster.org/17424 Smoke: Gluster Build System <jenkins@build.gluster.org> NetBSD-regression: NetBSD Build System <jenkins@build.gluster.org> Reviewed-by: Raghavendra G <rgowdapp@redhat.com> CentOS-regression: Gluster Build System <jenkins@build.gluster.org>
-rw-r--r--xlators/performance/io-cache/src/ioc-inode.c78
1 files changed, 40 insertions, 38 deletions
diff --git a/xlators/performance/io-cache/src/ioc-inode.c b/xlators/performance/io-cache/src/ioc-inode.c
index cee3bad8c22..6eb34124d1f 100644
--- a/xlators/performance/io-cache/src/ioc-inode.c
+++ b/xlators/performance/io-cache/src/ioc-inode.c
@@ -83,47 +83,45 @@ ioc_inode_wakeup (call_frame_t *frame, ioc_inode_t *ioc_inode,
goto out;
}
- ioc_inode_lock (ioc_inode);
- {
- waiter = ioc_inode->waitq;
- ioc_inode->waitq = NULL;
- }
- ioc_inode_unlock (ioc_inode);
-
if (stbuf)
cache_still_valid = ioc_cache_still_valid (ioc_inode, stbuf);
else
cache_still_valid = 0;
- if (!waiter) {
- gf_msg (frame->this->name, GF_LOG_WARNING, 0,
- IO_CACHE_MSG_PAGE_WAIT_VALIDATE,
- "cache validate called without any "
- "page waiting to be validated");
- }
+ ioc_inode_lock (ioc_inode);
+ {
+
+ waiter = ioc_inode->waitq;
+ if (!waiter) {
+ gf_msg (frame->this->name, GF_LOG_WARNING, 0,
+ IO_CACHE_MSG_PAGE_WAIT_VALIDATE,
+ "cache validate called without any "
+ "page waiting to be validated");
+
+ ioc_inode_unlock (ioc_inode);
+ goto out;
+ }
- while (waiter) {
- waiter_page = waiter->data;
- page_waitq = NULL;
+ while (waiter) {
+ waiter_page = waiter->data;
+ ioc_inode->waitq = waiter->next;
+ page_waitq = NULL;
- if (waiter_page) {
- if (cache_still_valid) {
- /* cache valid, wake up page */
- ioc_inode_lock (ioc_inode);
- {
+ if (waiter_page) {
+ if (cache_still_valid) {
+ /* cache valid, wake up page */
page_waitq =
__ioc_page_wakeup (waiter_page,
- waiter_page->op_errno);
- }
- ioc_inode_unlock (ioc_inode);
- if (page_waitq)
- ioc_waitq_return (page_waitq);
- } else {
+ waiter_page->op_errno);
+ if (page_waitq) {
+ ioc_inode_unlock (ioc_inode);
+ ioc_waitq_return (page_waitq);
+ ioc_inode_lock (ioc_inode);
+ }
+ } else {
/* cache invalid, generate page fault and set
* page->ready = 0, to avoid double faults
*/
- ioc_inode_lock (ioc_inode);
- {
if (waiter_page->ready) {
waiter_page->ready = 0;
need_fault = 1;
@@ -138,24 +136,28 @@ ioc_inode_wakeup (call_frame_t *frame, ioc_inode_t *ioc_inode,
frame,
waiter_page);
}
- }
- ioc_inode_unlock (ioc_inode);
- if (need_fault) {
- need_fault = 0;
- ioc_page_fault (ioc_inode, frame,
- local->fd,
- waiter_page->offset);
+
+ if (need_fault) {
+ need_fault = 0;
+ ioc_inode_unlock (ioc_inode);
+ ioc_page_fault (ioc_inode,
+ frame,
+ local->fd,
+ waiter_page->offset);
+ ioc_inode_lock (ioc_inode);
+ }
}
}
- }
waited = waiter;
- waiter = waiter->next;
+ waiter = ioc_inode->waitq;
waited->data = NULL;
GF_FREE (waited);
+ }
}
+ ioc_inode_unlock (ioc_inode);
out:
return;