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]  <IRQ>  [<ffffffff8103a3ef>] warn_slowpath_common+0x7f/0xc0
[65357.336893]  [<ffffffff8103a44a>] warn_slowpath_null+0x1a/0x20
[65357.354368]  [<ffffffffa0ff17b8>] lttng_event_write+0x118/0x140 [lttng_ring_buffer_client_mmap_overwrite]
[65357.383025]  [<ffffffffa100134f>] __event_probe__block_rq_with_error+0x1bf/0x220 [lttng_probe_block]
[65357.410376]  [<ffffffff812ea134>] blk_update_request+0x324/0x720
[65357.428364]  [<ffffffff812ea561>] blk_update_bidi_request+0x31/0x90
[65357.447136]  [<ffffffff812eb68c>] blk_end_bidi_request+0x2c/0x80
[65357.465127]  [<ffffffff812eb6f0>] blk_end_request+0x10/0x20
[65357.481822]  [<ffffffff81406b7c>] scsi_io_completion+0x9c/0x670
[65357.499555]  [<ffffffff813fe320>] scsi_finish_command+0xb0/0xe0
[65357.517283]  [<ffffffff81406965>] scsi_softirq_done+0xa5/0x140
[65357.534758]  [<ffffffff812f1d30>] blk_done_softirq+0x80/0xa0
[65357.551710]  [<ffffffff81043b00>] __do_softirq+0xe0/0x440
[65357.567881]  [<ffffffff81043ffe>] irq_exit+0x9e/0xb0
[65357.582754]  [<ffffffff81026465>] smp_call_function_single_interrupt+0x35/0x40
[65357.604388]  [<ffffffff8167be2f>] call_function_single_interrupt+0x6f/0x80
[65357.624976]  <EOI>  [<ffffffff8100ac06>] ? default_idle+0x46/0x300
[65357.643541]  [<ffffffff8100ac04>] ? default_idle+0x44/0x300
[65357.660235]  [<ffffffff8100b899>] cpu_idle+0x89/0xe0
[65357.675109]  [<ffffffff81664911>] 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 <mathieu.desnoyers@efficios.com>

diff --git a/lib/ringbuffer/backend.h b/lib/ringbuffer/backend.h
index 826be933fe61bb45efbe96a7738823e3ae46af9e..b5c4fb264e4203ee72aef594705d04b18da7e4bd 100644 (file)
--- 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 69ad2a709a1d8727862ab1e541438cc616c1f7a3..8c9d2b7c54e7b583e896fec159349b2336a0fb21 100644 (file)
--- 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);