From d2bd780e91af600f4b0126b5ff4f560a44a4a32e Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Sun, 30 Jun 2013 17:38:50 -0400 Subject: [PATCH] Fix: handle writes of length 0 lib_ring_buffer_write(), lib_ring_buffer_memset() and lib_ring_buffer_copy_from_user_inatomic() could be passed a length of 0. This typically has no side-effect as far as writing into the buffers is concerned, except for one detail: in overwrite mode, there is a check to make sure the sub-buffer can be written into. This check is performed even if length is 0. In the case where this would fall exactly at the end of a sub-buffer, the check would fail, because the offset would fall exactly at the beginning of the next sub-buffer. It triggers this warning: [65356.890016] ------------[ cut here ]------------ [65356.890016] WARNING: at /home/compudj/git/lttng-modules/wrapper/ringbuffer/../../lib/ringbuffer/../../wrapper/ringbuffer/../../lib/ringbuffer/backend.h:110 lttng_event_write+0x118/0x140 [lttng_ring_buffer_client_mmap_overwrite]() [65356.890016] Hardware name: X7DAL [65356.890016] Modules linked in: lttng_probe_writeback(O) lttng_probe_workqueue(O) lttng_probe_vmscan(O) lttng_probe_udp(O) lttng_probe_timer(O) lttng_probe_sunrpc(O) lttng_probe_statedump(O) lttng_probe_sock(O) lttng_probe_skb(O) lttng_probe_signal(O) lttng_probe_scsi(O) lttng_probe_sched(O) lttng_probe_rcu(O) lttng_probe_random(O) lttng_probe_printk(O) lttng_probe_power(O) lttng_probe_net(O) lttng_probe_napi(O) lttng_probe_module(O) lttng_probe_kvm(O) lttng_probe_kmem(O) lttng_probe_jbd2(O) lttng_probe_jbd(O) lttng_probe_irq(O) lttng_probe_ext4(O) lttng_probe_ext3(O) lttng_probe_compaction(O) lttng_probe_btrfs(O) lttng_probe_block(O) lttng_types(O) lttng_ring_buffer_metadata_mmap_client(O) lttng_ring_buffer_client_mmap_overwrite(O) lttng_ring_buffer_client_mmap_discard(O) lttng_ring_buffer_metadata_client(O) lttng_ring_buffer_client_overwrite(O) lttng_ring_buffer_client_discard(O) lttng_tracer(O) lttng_kretprobes(O) lttng_ftrace(O) lttng_kprobes(O) lttng_statedump(O) lttng_lib_ring_buffer(O) cpufreq_ondemand loop e1000e kvm_intel kvm ptp pps_core [last unloaded: lttng_lib_ring_buffer] [65357.287529] Pid: 0, comm: swapper/7 Tainted: G O 3.9.4-trace-test #143 [65357.309694] Call Trace: [65357.317022] [] warn_slowpath_common+0x7f/0xc0 [65357.336893] [] warn_slowpath_null+0x1a/0x20 [65357.354368] [] lttng_event_write+0x118/0x140 [lttng_ring_buffer_client_mmap_overwrite] [65357.383025] [] __event_probe__block_rq_with_error+0x1bf/0x220 [lttng_probe_block] [65357.410376] [] blk_update_request+0x324/0x720 [65357.428364] [] blk_update_bidi_request+0x31/0x90 [65357.447136] [] blk_end_bidi_request+0x2c/0x80 [65357.465127] [] blk_end_request+0x10/0x20 [65357.481822] [] scsi_io_completion+0x9c/0x670 [65357.499555] [] scsi_finish_command+0xb0/0xe0 [65357.517283] [] scsi_softirq_done+0xa5/0x140 [65357.534758] [] blk_done_softirq+0x80/0xa0 [65357.551710] [] __do_softirq+0xe0/0x440 [65357.567881] [] irq_exit+0x9e/0xb0 [65357.582754] [] smp_call_function_single_interrupt+0x35/0x40 [65357.604388] [] call_function_single_interrupt+0x6f/0x80 [65357.624976] [] ? default_idle+0x46/0x300 [65357.643541] [] ? default_idle+0x44/0x300 [65357.660235] [] cpu_idle+0x89/0xe0 [65357.675109] [] start_secondary+0x220/0x227 Always from an event that can write a 0-length field as last field of its payload, and it always happen directly on a sub-buffer boundary. While we are there, check for length 0 in lib_ring_buffer_read_cstr() too. Signed-off-by: Mathieu Desnoyers --- lib/ringbuffer/backend.h | 6 ++++++ lib/ringbuffer/ring_buffer_backend.c | 5 ++++- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/lib/ringbuffer/backend.h b/lib/ringbuffer/backend.h index 826be933..b5c4fb26 100644 --- a/lib/ringbuffer/backend.h +++ b/lib/ringbuffer/backend.h @@ -96,6 +96,8 @@ void lib_ring_buffer_write(const struct lib_ring_buffer_config *config, struct lib_ring_buffer_backend_pages *rpages; unsigned long sb_bindex, id; + if (unlikely(!len)) + return; offset &= chanb->buf_size - 1; sbidx = offset >> chanb->subbuf_size_order; index = (offset & (chanb->subbuf_size - 1)) >> PAGE_SHIFT; @@ -142,6 +144,8 @@ void lib_ring_buffer_memset(const struct lib_ring_buffer_config *config, struct lib_ring_buffer_backend_pages *rpages; unsigned long sb_bindex, id; + if (unlikely(!len)) + return; offset &= chanb->buf_size - 1; sbidx = offset >> chanb->subbuf_size_order; index = (offset & (chanb->subbuf_size - 1)) >> PAGE_SHIFT; @@ -189,6 +193,8 @@ void lib_ring_buffer_copy_from_user_inatomic(const struct lib_ring_buffer_config unsigned long ret; mm_segment_t old_fs = get_fs(); + if (unlikely(!len)) + return; offset &= chanb->buf_size - 1; sbidx = offset >> chanb->subbuf_size_order; index = (offset & (chanb->subbuf_size - 1)) >> PAGE_SHIFT; diff --git a/lib/ringbuffer/ring_buffer_backend.c b/lib/ringbuffer/ring_buffer_backend.c index 69ad2a70..8c9d2b7c 100644 --- a/lib/ringbuffer/ring_buffer_backend.c +++ b/lib/ringbuffer/ring_buffer_backend.c @@ -724,8 +724,9 @@ EXPORT_SYMBOL_GPL(__lib_ring_buffer_copy_to_user); * @dest : destination address * @len : destination's length * - * return string's length + * Return string's length, or -EINVAL on error. * Should be protected by get_subbuf/put_subbuf. + * Destination length should be at least 1 to hold '\0'. */ int lib_ring_buffer_read_cstr(struct lib_ring_buffer_backend *bufb, size_t offset, void *dest, size_t len) @@ -741,6 +742,8 @@ int lib_ring_buffer_read_cstr(struct lib_ring_buffer_backend *bufb, size_t offse offset &= chanb->buf_size - 1; index = (offset & (chanb->subbuf_size - 1)) >> PAGE_SHIFT; orig_offset = offset; + if (unlikely(!len)) + return -EINVAL; for (;;) { id = bufb->buf_rsb.id; sb_bindex = subbuffer_id_get_index(config, id); -- 2.34.1