From 3a161d99c43ce74c76aecff309be4c3ba455e823 Mon Sep 17 00:00:00 2001 From: "Steven Rostedt (Red Hat)" Date: Wed, 25 Jun 2014 15:54:42 -0400 Subject: tracing: Create seq_buf layer in trace_seq Create a seq_buf layer that trace_seq sits on. The seq_buf will not be limited to page size. This will allow other usages of seq_buf instead of a hard set PAGE_SIZE one that trace_seq has. Link: http://lkml.kernel.org/r/20141104160221.864997179@goodmis.org Link: http://lkml.kernel.org/r/20141114011412.170377300@goodmis.org Tested-by: Jiri Kosina Acked-by: Jiri Kosina Reviewed-by: Petr Mladek Signed-off-by: Steven Rostedt --- kernel/trace/trace.c | 39 +++++++++++++++++++++------------------ 1 file changed, 21 insertions(+), 18 deletions(-) (limited to 'kernel/trace/trace.c') diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 3ce3c4ccfc94..7d7a07e9b9e9 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -939,19 +939,20 @@ out: return ret; } +/* TODO add a seq_buf_to_buffer() */ static ssize_t trace_seq_to_buffer(struct trace_seq *s, void *buf, size_t cnt) { int len; - if (s->len <= s->readpos) + if (s->seq.len <= s->seq.readpos) return -EBUSY; - len = s->len - s->readpos; + len = s->seq.len - s->seq.readpos; if (cnt > len) cnt = len; - memcpy(buf, s->buffer + s->readpos, cnt); + memcpy(buf, s->buffer + s->seq.readpos, cnt); - s->readpos += cnt; + s->seq.readpos += cnt; return cnt; } @@ -4315,6 +4316,8 @@ static int tracing_open_pipe(struct inode *inode, struct file *filp) goto out; } + trace_seq_init(&iter->seq); + /* * We make a copy of the current tracer to avoid concurrent * changes on it while we are reading. @@ -4511,18 +4514,18 @@ waitagain: trace_access_lock(iter->cpu_file); while (trace_find_next_entry_inc(iter) != NULL) { enum print_line_t ret; - int len = iter->seq.len; + int len = iter->seq.seq.len; ret = print_trace_line(iter); if (ret == TRACE_TYPE_PARTIAL_LINE) { /* don't print partial lines */ - iter->seq.len = len; + iter->seq.seq.len = len; break; } if (ret != TRACE_TYPE_NO_CONSUME) trace_consume(iter); - if (iter->seq.len >= cnt) + if (iter->seq.seq.len >= cnt) break; /* @@ -4538,7 +4541,7 @@ waitagain: /* Now copy what we have to the user */ sret = trace_seq_to_user(&iter->seq, ubuf, cnt); - if (iter->seq.readpos >= iter->seq.len) + if (iter->seq.seq.readpos >= iter->seq.seq.len) trace_seq_init(&iter->seq); /* @@ -4576,16 +4579,16 @@ tracing_fill_pipe_page(size_t rem, struct trace_iterator *iter) /* Seq buffer is page-sized, exactly what we need. */ for (;;) { - count = iter->seq.len; + count = iter->seq.seq.len; ret = print_trace_line(iter); - count = iter->seq.len - count; + count = iter->seq.seq.len - count; if (rem < count) { rem = 0; - iter->seq.len -= count; + iter->seq.seq.len -= count; break; } if (ret == TRACE_TYPE_PARTIAL_LINE) { - iter->seq.len -= count; + iter->seq.seq.len -= count; break; } @@ -4666,13 +4669,13 @@ static ssize_t tracing_splice_read_pipe(struct file *filp, /* Copy the data into the page, so we can start over. */ ret = trace_seq_to_buffer(&iter->seq, page_address(spd.pages[i]), - iter->seq.len); + iter->seq.seq.len); if (ret < 0) { __free_page(spd.pages[i]); break; } spd.partial[i].offset = 0; - spd.partial[i].len = iter->seq.len; + spd.partial[i].len = iter->seq.seq.len; trace_seq_init(&iter->seq); } @@ -5673,7 +5676,7 @@ tracing_stats_read(struct file *filp, char __user *ubuf, cnt = ring_buffer_read_events_cpu(trace_buf->buffer, cpu); trace_seq_printf(s, "read events: %ld\n", cnt); - count = simple_read_from_buffer(ubuf, count, ppos, s->buffer, s->len); + count = simple_read_from_buffer(ubuf, count, ppos, s->buffer, s->seq.len); kfree(s); @@ -6636,11 +6639,11 @@ void trace_printk_seq(struct trace_seq *s) { /* Probably should print a warning here. */ - if (s->len >= TRACE_MAX_PRINT) - s->len = TRACE_MAX_PRINT; + if (s->seq.len >= TRACE_MAX_PRINT) + s->seq.len = TRACE_MAX_PRINT; /* should be zero ended, but we are paranoid. */ - s->buffer[s->len] = 0; + s->buffer[s->seq.len] = 0; printk(KERN_TRACE "%s", s->buffer); -- cgit v1.2.3 From 74f06bb72347302a19aac087314388ebd0e4fee9 Mon Sep 17 00:00:00 2001 From: "Steven Rostedt (Red Hat)" Date: Mon, 17 Nov 2014 13:12:22 -0500 Subject: tracing: Clean up tracing_fill_pipe_page() The function tracing_fill_pipe_page() logic is a little confusing with the use of count saving the seq.len and reusing it. Instead of subtracting a number that is calculated from the saved value of the seq.len from seq.len, just save the seq.len at the start and if we need to reset it, just assign it again. When the seq_buf overflow is len == size + 1, the current logic will break. Changing it to use a saved length for resetting back to the original value is more robust and will work when we change the way seq_buf sets the overflow. Link: http://lkml.kernel.org/r/20141118161546.GJ23958@pathway.suse.cz Reviewed-by: Petr Mladek Signed-off-by: Steven Rostedt --- kernel/trace/trace.c | 25 +++++++++++++++++++------ 1 file changed, 19 insertions(+), 6 deletions(-) (limited to 'kernel/trace/trace.c') diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 7d7a07e9b9e9..0aa75be843a0 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -4575,20 +4575,33 @@ static size_t tracing_fill_pipe_page(size_t rem, struct trace_iterator *iter) { size_t count; + int save_len; int ret; /* Seq buffer is page-sized, exactly what we need. */ for (;;) { - count = iter->seq.seq.len; + save_len = iter->seq.seq.len; ret = print_trace_line(iter); - count = iter->seq.seq.len - count; - if (rem < count) { - rem = 0; - iter->seq.seq.len -= count; + + if (trace_seq_has_overflowed(&iter->seq)) { + iter->seq.seq.len = save_len; break; } + + /* + * This should not be hit, because it should only + * be set if the iter->seq overflowed. But check it + * anyway to be safe. + */ if (ret == TRACE_TYPE_PARTIAL_LINE) { - iter->seq.seq.len -= count; + iter->seq.seq.len = save_len; + break; + } + + count = iter->seq.seq.len - save_len; + if (rem < count) { + rem = 0; + iter->seq.seq.len = save_len; break; } -- cgit v1.2.3 From 5ac48378414dccca735897c4d7f4e19987c8977c Mon Sep 17 00:00:00 2001 From: "Steven Rostedt (Red Hat)" Date: Fri, 14 Nov 2014 15:49:41 -0500 Subject: tracing: Use trace_seq_used() and seq_buf_used() instead of len As the seq_buf->len will soon be +1 size when there's an overflow, we must use trace_seq_used() or seq_buf_used() methods to get the real length. This will prevent buffer overflow issues if just the len of the seq_buf descriptor is used to copy memory. Link: http://lkml.kernel.org/r/20141114121911.09ba3d38@gandalf.local.home Reported-by: Petr Mladek Signed-off-by: Steven Rostedt --- kernel/trace/trace.c | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) (limited to 'kernel/trace/trace.c') diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 0aa75be843a0..9023446b2c2b 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -944,10 +944,10 @@ static ssize_t trace_seq_to_buffer(struct trace_seq *s, void *buf, size_t cnt) { int len; - if (s->seq.len <= s->seq.readpos) + if (trace_seq_used(s) <= s->seq.readpos) return -EBUSY; - len = s->seq.len - s->seq.readpos; + len = trace_seq_used(s) - s->seq.readpos; if (cnt > len) cnt = len; memcpy(buf, s->buffer + s->seq.readpos, cnt); @@ -4514,18 +4514,18 @@ waitagain: trace_access_lock(iter->cpu_file); while (trace_find_next_entry_inc(iter) != NULL) { enum print_line_t ret; - int len = iter->seq.seq.len; + int save_len = iter->seq.seq.len; ret = print_trace_line(iter); if (ret == TRACE_TYPE_PARTIAL_LINE) { /* don't print partial lines */ - iter->seq.seq.len = len; + iter->seq.seq.len = save_len; break; } if (ret != TRACE_TYPE_NO_CONSUME) trace_consume(iter); - if (iter->seq.seq.len >= cnt) + if (trace_seq_used(&iter->seq) >= cnt) break; /* @@ -4541,7 +4541,7 @@ waitagain: /* Now copy what we have to the user */ sret = trace_seq_to_user(&iter->seq, ubuf, cnt); - if (iter->seq.seq.readpos >= iter->seq.seq.len) + if (iter->seq.seq.readpos >= trace_seq_used(&iter->seq)) trace_seq_init(&iter->seq); /* @@ -4598,7 +4598,7 @@ tracing_fill_pipe_page(size_t rem, struct trace_iterator *iter) break; } - count = iter->seq.seq.len - save_len; + count = trace_seq_used(&iter->seq) - save_len; if (rem < count) { rem = 0; iter->seq.seq.len = save_len; @@ -4682,13 +4682,13 @@ static ssize_t tracing_splice_read_pipe(struct file *filp, /* Copy the data into the page, so we can start over. */ ret = trace_seq_to_buffer(&iter->seq, page_address(spd.pages[i]), - iter->seq.seq.len); + trace_seq_used(&iter->seq)); if (ret < 0) { __free_page(spd.pages[i]); break; } spd.partial[i].offset = 0; - spd.partial[i].len = iter->seq.seq.len; + spd.partial[i].len = trace_seq_used(&iter->seq); trace_seq_init(&iter->seq); } @@ -5689,7 +5689,8 @@ tracing_stats_read(struct file *filp, char __user *ubuf, cnt = ring_buffer_read_events_cpu(trace_buf->buffer, cpu); trace_seq_printf(s, "read events: %ld\n", cnt); - count = simple_read_from_buffer(ubuf, count, ppos, s->buffer, s->seq.len); + count = simple_read_from_buffer(ubuf, count, ppos, + s->buffer, trace_seq_used(s)); kfree(s); -- cgit v1.2.3 From 820b75f63d0152dbb9ff4accf274408592d613f2 Mon Sep 17 00:00:00 2001 From: "Steven Rostedt (Red Hat)" Date: Wed, 19 Nov 2014 10:56:41 -0500 Subject: tracing: Add paranoid size check in trace_printk_seq() To be really paranoid about writing out of bound data in trace_printk_seq(), add another check of len compared to size. Link: http://lkml.kernel.org/r/20141119144004.GB2332@dhcp128.suse.cz Suggested-by: Petr Mladek Reviewed-by: Petr Mladek Signed-off-by: Steven Rostedt --- kernel/trace/trace.c | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'kernel/trace/trace.c') diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c index 9023446b2c2b..26facec4625e 100644 --- a/kernel/trace/trace.c +++ b/kernel/trace/trace.c @@ -6656,6 +6656,14 @@ trace_printk_seq(struct trace_seq *s) if (s->seq.len >= TRACE_MAX_PRINT) s->seq.len = TRACE_MAX_PRINT; + /* + * More paranoid code. Although the buffer size is set to + * PAGE_SIZE, and TRACE_MAX_PRINT is 1000, this is just + * an extra layer of protection. + */ + if (WARN_ON_ONCE(s->seq.len >= s->seq.size)) + s->seq.len = s->seq.size - 1; + /* should be zero ended, but we are paranoid. */ s->buffer[s->seq.len] = 0; -- cgit v1.2.3