diff options
author | Chunwei Chen <[email protected]> | 2018-03-12 11:27:02 -0700 |
---|---|---|
committer | Brian Behlendorf <[email protected]> | 2018-03-12 11:27:02 -0700 |
commit | 9470cbd4f95ecd730d18f7dadd56bcf54c6711b0 (patch) | |
tree | c6088ab0720a1812380e938a86fa80077d1dccd0 /module/zfs | |
parent | b7eec00f9fc05e883b0dadb231251b5996d02e3e (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/zfs')
-rw-r--r-- | module/zfs/zrlock.c | 6 |
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; |