summaryrefslogtreecommitdiffstats
path: root/module
diff options
context:
space:
mode:
authorChunwei Chen <[email protected]>2018-03-12 11:27:02 -0700
committerBrian Behlendorf <[email protected]>2018-03-12 11:27:02 -0700
commit9470cbd4f95ecd730d18f7dadd56bcf54c6711b0 (patch)
treec6088ab0720a1812380e938a86fa80077d1dccd0 /module
parentb7eec00f9fc05e883b0dadb231251b5996d02e3e (diff)
Fix race in trace point in zrl_add_impl
We hit an illegal memory access in the zrlock trace point. The problem is that zrl->zr_owner and zrl->zr_caller are assigned locklessly. And if zrl->zr_owner got assigned a longer string between when __string() calculate the strlen, and when __assign_str() does strcpy. The copy will overflow the buffer. == For example: Initial condition: zrl->zr_owner = A zrl->zr_caller = "abc" Thread A Thread B ------------------------------------------------- if (zrl->zr_owner == A) { DTRACE_PROBE2() { __string() { strlen(zrl->zr_caller) -> 3 allocate buf[4] } zrl->zr_owner = B zrl->zr_caller = "abcd" __assign_str() { strcpy(buf, zrl->zr_caller) <- buffer overflow == Dereferencing zrl->zr_owner->pid may also be problematic, in that the zrl->zr_owner got changed to other task, and that task exits, freeing the task_struct. This should be very unlikely, as the other task need to zrl_remove and exit between the dereferencing zr->zr_owner and zr->zr_owner->pid. Nevertheless, we'll deal with it as well. To fix the zrl->zr_caller issue, instead of copy the string content, we just copy the pointer, this is safe because it always points to __func__, which is static. As for the zrl->zr_owner issue, we pass in curthread instead of using zrl->zr_owner. Reviewed-by: Brian Behlendorf <[email protected]> Signed-off-by: Chunwei Chen <[email protected]> Closes #7291
Diffstat (limited to 'module')
-rw-r--r--module/zfs/zrlock.c6
1 files changed, 4 insertions, 2 deletions
diff --git a/module/zfs/zrlock.c b/module/zfs/zrlock.c
index 167d4c506..4f4854436 100644
--- a/module/zfs/zrlock.c
+++ b/module/zfs/zrlock.c
@@ -82,8 +82,10 @@ zrl_add_impl(zrlock_t *zrl, const char *zc)
ASSERT3S((int32_t)n, >=, 0);
#ifdef ZFS_DEBUG
if (zrl->zr_owner == curthread) {
- DTRACE_PROBE2(zrlock__reentry,
- zrlock_t *, zrl, uint32_t, n);
+ DTRACE_PROBE3(zrlock__reentry,
+ zrlock_t *, zrl,
+ kthread_t *, curthread,
+ uint32_t, n);
}
zrl->zr_owner = curthread;
zrl->zr_caller = zc;