From: Mathieu Desnoyers Date: Wed, 22 Aug 2012 20:30:20 +0000 (-0400) Subject: Fix: ensure userspace accesses are done with _inatomic X-Git-Tag: v2.1.0-rc1~11 X-Git-Url: http://git.lttng.org/?p=lttng-modules.git;a=commitdiff_plain;h=7b8ea3a503536c9d00f8bb14958a150cbf7370c0 Fix: ensure userspace accesses are done with _inatomic Otherwise, triggers scheduling while atomic (might_sleep()) warnings, since we call those from a tracepoint probe (with preemption disabled). Signed-off-by: Mathieu Desnoyers --- diff --git a/Makefile b/Makefile index d06e6764..b91113e4 100644 --- a/Makefile +++ b/Makefile @@ -26,7 +26,7 @@ obj-m += lttng-statedump.o lttng-statedump-objs := lttng-statedump-impl.o wrapper/irqdesc.o ifneq ($(CONFIG_HAVE_SYSCALL_TRACEPOINTS),) -lttng-tracer-objs += lttng-syscalls.o +lttng-tracer-objs += lttng-syscalls.o probes/lttng-probe-user.o endif ifneq ($(CONFIG_PERF_EVENTS),) diff --git a/lib/ringbuffer/backend.h b/lib/ringbuffer/backend.h index 2ce6ce9c..826be933 100644 --- a/lib/ringbuffer/backend.h +++ b/lib/ringbuffer/backend.h @@ -34,6 +34,7 @@ #include #include #include +#include /* Internal helpers */ #include "../../wrapper/ringbuffer/backend_internal.h" @@ -161,7 +162,7 @@ void lib_ring_buffer_memset(const struct lib_ring_buffer_config *config, } /** - * lib_ring_buffer_copy_from_user - write userspace data to a buffer backend + * lib_ring_buffer_copy_from_user_inatomic - write userspace data to a buffer backend * @config : ring buffer instance configuration * @ctx: ring buffer context. (input arguments only) * @src : userspace source pointer to copy from @@ -170,10 +171,11 @@ void lib_ring_buffer_memset(const struct lib_ring_buffer_config *config, * This function copies "len" bytes of data from a userspace pointer to a * buffer backend, at the current context offset. This is more or less a buffer * backend-specific memcpy() operation. Calls the slow path - * (_ring_buffer_write_from_user) if copy is crossing a page boundary. + * (_ring_buffer_write_from_user_inatomic) if copy is crossing a page boundary. + * Disable the page fault handler to ensure we never try to take the mmap_sem. */ static inline -void lib_ring_buffer_copy_from_user(const struct lib_ring_buffer_config *config, +void lib_ring_buffer_copy_from_user_inatomic(const struct lib_ring_buffer_config *config, struct lib_ring_buffer_ctx *ctx, const void __user *src, size_t len) { @@ -185,6 +187,7 @@ void lib_ring_buffer_copy_from_user(const struct lib_ring_buffer_config *config, struct lib_ring_buffer_backend_pages *rpages; unsigned long sb_bindex, id; unsigned long ret; + mm_segment_t old_fs = get_fs(); offset &= chanb->buf_size - 1; sbidx = offset >> chanb->subbuf_size_order; @@ -197,11 +200,13 @@ void lib_ring_buffer_copy_from_user(const struct lib_ring_buffer_config *config, config->mode == RING_BUFFER_OVERWRITE && subbuffer_id_is_noref(config, id)); + set_fs(KERNEL_DS); + pagefault_disable(); if (unlikely(!access_ok(VERIFY_READ, src, len))) goto fill_buffer; if (likely(pagecpy == len)) { - ret = lib_ring_buffer_do_copy_from_user( + ret = lib_ring_buffer_do_copy_from_user_inatomic( rpages->p[index].virt + (offset & ~PAGE_MASK), src, len); if (unlikely(ret > 0)) { @@ -210,13 +215,17 @@ void lib_ring_buffer_copy_from_user(const struct lib_ring_buffer_config *config, goto fill_buffer; } } else { - _lib_ring_buffer_copy_from_user(bufb, offset, src, len, 0); + _lib_ring_buffer_copy_from_user_inatomic(bufb, offset, src, len, 0); } + pagefault_enable(); + set_fs(old_fs); ctx->buf_offset += len; return; fill_buffer: + pagefault_enable(); + set_fs(old_fs); /* * In the error path we call the slow path version to avoid * the pollution of static inline code. diff --git a/lib/ringbuffer/backend_internal.h b/lib/ringbuffer/backend_internal.h index 3e262c2e..e048365e 100644 --- a/lib/ringbuffer/backend_internal.h +++ b/lib/ringbuffer/backend_internal.h @@ -56,7 +56,7 @@ extern void _lib_ring_buffer_write(struct lib_ring_buffer_backend *bufb, extern void _lib_ring_buffer_memset(struct lib_ring_buffer_backend *bufb, size_t offset, int c, size_t len, ssize_t pagecpy); -extern void _lib_ring_buffer_copy_from_user(struct lib_ring_buffer_backend *bufb, +extern void _lib_ring_buffer_copy_from_user_inatomic(struct lib_ring_buffer_backend *bufb, size_t offset, const void *src, size_t len, ssize_t pagecpy); @@ -434,15 +434,15 @@ do { \ } while (0) /* - * We use __copy_from_user to copy userspace data since we already + * We use __copy_from_user_inatomic to copy userspace data since we already * did the access_ok for the whole range. */ static inline -unsigned long lib_ring_buffer_do_copy_from_user(void *dest, +unsigned long lib_ring_buffer_do_copy_from_user_inatomic(void *dest, const void __user *src, unsigned long len) { - return __copy_from_user(dest, src, len); + return __copy_from_user_inatomic(dest, src, len); } /* diff --git a/lib/ringbuffer/ring_buffer_backend.c b/lib/ringbuffer/ring_buffer_backend.c index e8ad4c52..84e7dfb7 100644 --- a/lib/ringbuffer/ring_buffer_backend.c +++ b/lib/ringbuffer/ring_buffer_backend.c @@ -559,7 +559,7 @@ EXPORT_SYMBOL_GPL(_lib_ring_buffer_memset); /** - * lib_ring_buffer_copy_from_user - write user data to a ring_buffer buffer. + * lib_ring_buffer_copy_from_user_inatomic - write user data to a ring_buffer buffer. * @bufb : buffer backend * @offset : offset within the buffer * @src : source address @@ -570,7 +570,7 @@ EXPORT_SYMBOL_GPL(_lib_ring_buffer_memset); * directly without having the src pointer checked with access_ok() * previously. */ -void _lib_ring_buffer_copy_from_user(struct lib_ring_buffer_backend *bufb, +void _lib_ring_buffer_copy_from_user_inatomic(struct lib_ring_buffer_backend *bufb, size_t offset, const void __user *src, size_t len, ssize_t pagecpy) @@ -601,7 +601,7 @@ void _lib_ring_buffer_copy_from_user(struct lib_ring_buffer_backend *bufb, rpages = bufb->array[sb_bindex]; CHAN_WARN_ON(chanb, config->mode == RING_BUFFER_OVERWRITE && subbuffer_id_is_noref(config, id)); - ret = lib_ring_buffer_do_copy_from_user(rpages->p[index].virt + ret = lib_ring_buffer_do_copy_from_user_inatomic(rpages->p[index].virt + (offset & ~PAGE_MASK), src, pagecpy) != 0; if (ret > 0) { @@ -612,7 +612,7 @@ void _lib_ring_buffer_copy_from_user(struct lib_ring_buffer_backend *bufb, } } while (unlikely(len != pagecpy)); } -EXPORT_SYMBOL_GPL(_lib_ring_buffer_copy_from_user); +EXPORT_SYMBOL_GPL(_lib_ring_buffer_copy_from_user_inatomic); /** * lib_ring_buffer_read - read data from ring_buffer_buffer. diff --git a/lttng-ring-buffer-client.h b/lttng-ring-buffer-client.h index 9016b438..cf2645a5 100644 --- a/lttng-ring-buffer-client.h +++ b/lttng-ring-buffer-client.h @@ -513,7 +513,7 @@ static void lttng_event_write_from_user(struct lib_ring_buffer_ctx *ctx, const void __user *src, size_t len) { - lib_ring_buffer_copy_from_user(&client_config, ctx, src, len); + lib_ring_buffer_copy_from_user_inatomic(&client_config, ctx, src, len); } static diff --git a/lttng-ring-buffer-metadata-client.h b/lttng-ring-buffer-metadata-client.h index 8660d997..1c77f99c 100644 --- a/lttng-ring-buffer-metadata-client.h +++ b/lttng-ring-buffer-metadata-client.h @@ -240,7 +240,7 @@ static void lttng_event_write_from_user(struct lib_ring_buffer_ctx *ctx, const void __user *src, size_t len) { - lib_ring_buffer_copy_from_user(&client_config, ctx, src, len); + lib_ring_buffer_copy_from_user_inatomic(&client_config, ctx, src, len); } static diff --git a/probes/lttng-events.h b/probes/lttng-events.h index d3d75ad5..33cabcf9 100644 --- a/probes/lttng-events.h +++ b/probes/lttng-events.h @@ -22,6 +22,7 @@ #include #include "lttng.h" #include "lttng-types.h" +#include "lttng-probe-user.h" #include "../wrapper/vmalloc.h" /* for wrapper_vmalloc_sync_all() */ #include "../wrapper/ringbuffer/frontend_types.h" #include "../lttng-events.h" @@ -359,7 +360,7 @@ static __used struct lttng_probe_desc TP_ID(__probe_desc___, TRACE_SYSTEM) = { #undef __string_from_user #define __string_from_user(_item, _src) \ __event_len += __dynamic_len[__dynamic_len_idx++] = \ - max_t(size_t, strlen_user(_src), 1); + max_t(size_t, lttng_strlen_user_inatomic(_src), 1); #undef TP_PROTO #define TP_PROTO(args...) args diff --git a/probes/lttng-probe-user.c b/probes/lttng-probe-user.c new file mode 100644 index 00000000..94ecf2f1 --- /dev/null +++ b/probes/lttng-probe-user.c @@ -0,0 +1,54 @@ +/* + * lttng-probe-user.c + * + * Copyright (C) 2012 Mathieu Desnoyers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; only + * version 2.1 of the License. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include +#include "lttng-probe-user.h" + +/* + * Calculate string length. Include final null terminating character if there is + * one, or ends at first fault. Disabling page faults ensures that we can safely + * call this from pretty much any context, including those where the caller + * holds mmap_sem, or any lock which nests in mmap_sem. + */ +long lttng_strlen_user_inatomic(const char *addr) +{ + long count = 0; + mm_segment_t old_fs = get_fs(); + + set_fs(KERNEL_DS); + pagefault_disable(); + for (;;) { + char v; + long ret; + + ret = __copy_from_user_inatomic(&v, + (__force const char __user *)(addr), + sizeof(v)); + if (unlikely(ret == -EFAULT)) + break; + count++; + if (unlikely(!v)) + break; + addr++; + } + pagefault_enable(); + set_fs(old_fs); + return count; +} diff --git a/probes/lttng-probe-user.h b/probes/lttng-probe-user.h new file mode 100644 index 00000000..e03e4b01 --- /dev/null +++ b/probes/lttng-probe-user.h @@ -0,0 +1,30 @@ +#ifndef _LTTNG_PROBE_USER_H +#define _LTTNG_PROBE_USER_H + +/* + * lttng-probe-user.h + * + * Copyright (C) 2012 Mathieu Desnoyers + * + * This library is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; only + * version 2.1 of the License. + * + * This library is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with this library; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +/* + * Calculate string length. Include final null terminating character if there is + * one, or ends at first fault. + */ +long lttng_strlen_user_inatomic(const char *addr); + +#endif /* _LTTNG_PROBE_USER_H */