summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorN Balachandran <nbalacha@redhat.com>2017-05-29 15:21:39 +0530
committerJeff Darcy <jeff@pl.atyp.us>2017-05-30 15:51:46 +0000
commit6b6162f7ff93bccef0e615cb490e881168827e1d (patch)
tree0b2eba88d1c7737c0ef1e6e78b7fcd2157bf4207
parentec86167d09bcbb763e31b73fb3d688efaa5444d7 (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 Change-Id: I54b064857e2694826d0c03b23f8014e3984a3330 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>
-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;