diff options
author | Steven Rostedt (Red Hat) <rostedt@goodmis.org> | 2014-11-14 15:49:41 -0500 |
---|---|---|
committer | Steven Rostedt <rostedt@goodmis.org> | 2014-11-19 22:01:15 -0500 |
commit | 5ac48378414dccca735897c4d7f4e19987c8977c (patch) | |
tree | 9cf5c11dddd8081327d7e7f8a68a9e47613adcfa | |
parent | 74f06bb72347302a19aac087314388ebd0e4fee9 (diff) |
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 <pmladek@suse.cz>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
-rw-r--r-- | include/linux/trace_seq.h | 20 | ||||
-rw-r--r-- | kernel/trace/seq_buf.c | 2 | ||||
-rw-r--r-- | kernel/trace/trace.c | 21 | ||||
-rw-r--r-- | kernel/trace/trace_events.c | 9 | ||||
-rw-r--r-- | kernel/trace/trace_functions_graph.c | 5 | ||||
-rw-r--r-- | kernel/trace/trace_seq.c | 2 |
6 files changed, 42 insertions, 17 deletions
diff --git a/include/linux/trace_seq.h b/include/linux/trace_seq.h index 85d37106be3d..cfaf5a1d4bad 100644 --- a/include/linux/trace_seq.h +++ b/include/linux/trace_seq.h | |||
@@ -24,6 +24,24 @@ trace_seq_init(struct trace_seq *s) | |||
24 | } | 24 | } |
25 | 25 | ||
26 | /** | 26 | /** |
27 | * trace_seq_used - amount of actual data written to buffer | ||
28 | * @s: trace sequence descriptor | ||
29 | * | ||
30 | * Returns the amount of data written to the buffer. | ||
31 | * | ||
32 | * IMPORTANT! | ||
33 | * | ||
34 | * Use this instead of @s->seq.len if you need to pass the amount | ||
35 | * of data from the buffer to another buffer (userspace, or what not). | ||
36 | * The @s->seq.len on overflow is bigger than the buffer size and | ||
37 | * using it can cause access to undefined memory. | ||
38 | */ | ||
39 | static inline int trace_seq_used(struct trace_seq *s) | ||
40 | { | ||
41 | return seq_buf_used(&s->seq); | ||
42 | } | ||
43 | |||
44 | /** | ||
27 | * trace_seq_buffer_ptr - return pointer to next location in buffer | 45 | * trace_seq_buffer_ptr - return pointer to next location in buffer |
28 | * @s: trace sequence descriptor | 46 | * @s: trace sequence descriptor |
29 | * | 47 | * |
@@ -35,7 +53,7 @@ trace_seq_init(struct trace_seq *s) | |||
35 | static inline unsigned char * | 53 | static inline unsigned char * |
36 | trace_seq_buffer_ptr(struct trace_seq *s) | 54 | trace_seq_buffer_ptr(struct trace_seq *s) |
37 | { | 55 | { |
38 | return s->buffer + s->seq.len; | 56 | return s->buffer + seq_buf_used(&s->seq); |
39 | } | 57 | } |
40 | 58 | ||
41 | /** | 59 | /** |
diff --git a/kernel/trace/seq_buf.c b/kernel/trace/seq_buf.c index 9ec5305d9da7..ce17f65268ed 100644 --- a/kernel/trace/seq_buf.c +++ b/kernel/trace/seq_buf.c | |||
@@ -328,7 +328,7 @@ int seq_buf_to_user(struct seq_buf *s, char __user *ubuf, int cnt) | |||
328 | if (s->len <= s->readpos) | 328 | if (s->len <= s->readpos) |
329 | return -EBUSY; | 329 | return -EBUSY; |
330 | 330 | ||
331 | len = s->len - s->readpos; | 331 | len = seq_buf_used(s) - s->readpos; |
332 | if (cnt > len) | 332 | if (cnt > len) |
333 | cnt = len; | 333 | cnt = len; |
334 | ret = copy_to_user(ubuf, s->buffer + s->readpos, cnt); | 334 | ret = copy_to_user(ubuf, s->buffer + s->readpos, cnt); |
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) | |||
944 | { | 944 | { |
945 | int len; | 945 | int len; |
946 | 946 | ||
947 | if (s->seq.len <= s->seq.readpos) | 947 | if (trace_seq_used(s) <= s->seq.readpos) |
948 | return -EBUSY; | 948 | return -EBUSY; |
949 | 949 | ||
950 | len = s->seq.len - s->seq.readpos; | 950 | len = trace_seq_used(s) - s->seq.readpos; |
951 | if (cnt > len) | 951 | if (cnt > len) |
952 | cnt = len; | 952 | cnt = len; |
953 | memcpy(buf, s->buffer + s->seq.readpos, cnt); | 953 | memcpy(buf, s->buffer + s->seq.readpos, cnt); |
@@ -4514,18 +4514,18 @@ waitagain: | |||
4514 | trace_access_lock(iter->cpu_file); | 4514 | trace_access_lock(iter->cpu_file); |
4515 | while (trace_find_next_entry_inc(iter) != NULL) { | 4515 | while (trace_find_next_entry_inc(iter) != NULL) { |
4516 | enum print_line_t ret; | 4516 | enum print_line_t ret; |
4517 | int len = iter->seq.seq.len; | 4517 | int save_len = iter->seq.seq.len; |
4518 | 4518 | ||
4519 | ret = print_trace_line(iter); | 4519 | ret = print_trace_line(iter); |
4520 | if (ret == TRACE_TYPE_PARTIAL_LINE) { | 4520 | if (ret == TRACE_TYPE_PARTIAL_LINE) { |
4521 | /* don't print partial lines */ | 4521 | /* don't print partial lines */ |
4522 | iter->seq.seq.len = len; | 4522 | iter->seq.seq.len = save_len; |
4523 | break; | 4523 | break; |
4524 | } | 4524 | } |
4525 | if (ret != TRACE_TYPE_NO_CONSUME) | 4525 | if (ret != TRACE_TYPE_NO_CONSUME) |
4526 | trace_consume(iter); | 4526 | trace_consume(iter); |
4527 | 4527 | ||
4528 | if (iter->seq.seq.len >= cnt) | 4528 | if (trace_seq_used(&iter->seq) >= cnt) |
4529 | break; | 4529 | break; |
4530 | 4530 | ||
4531 | /* | 4531 | /* |
@@ -4541,7 +4541,7 @@ waitagain: | |||
4541 | 4541 | ||
4542 | /* Now copy what we have to the user */ | 4542 | /* Now copy what we have to the user */ |
4543 | sret = trace_seq_to_user(&iter->seq, ubuf, cnt); | 4543 | sret = trace_seq_to_user(&iter->seq, ubuf, cnt); |
4544 | if (iter->seq.seq.readpos >= iter->seq.seq.len) | 4544 | if (iter->seq.seq.readpos >= trace_seq_used(&iter->seq)) |
4545 | trace_seq_init(&iter->seq); | 4545 | trace_seq_init(&iter->seq); |
4546 | 4546 | ||
4547 | /* | 4547 | /* |
@@ -4598,7 +4598,7 @@ tracing_fill_pipe_page(size_t rem, struct trace_iterator *iter) | |||
4598 | break; | 4598 | break; |
4599 | } | 4599 | } |
4600 | 4600 | ||
4601 | count = iter->seq.seq.len - save_len; | 4601 | count = trace_seq_used(&iter->seq) - save_len; |
4602 | if (rem < count) { | 4602 | if (rem < count) { |
4603 | rem = 0; | 4603 | rem = 0; |
4604 | iter->seq.seq.len = save_len; | 4604 | iter->seq.seq.len = save_len; |
@@ -4682,13 +4682,13 @@ static ssize_t tracing_splice_read_pipe(struct file *filp, | |||
4682 | /* Copy the data into the page, so we can start over. */ | 4682 | /* Copy the data into the page, so we can start over. */ |
4683 | ret = trace_seq_to_buffer(&iter->seq, | 4683 | ret = trace_seq_to_buffer(&iter->seq, |
4684 | page_address(spd.pages[i]), | 4684 | page_address(spd.pages[i]), |
4685 | iter->seq.seq.len); | 4685 | trace_seq_used(&iter->seq)); |
4686 | if (ret < 0) { | 4686 | if (ret < 0) { |
4687 | __free_page(spd.pages[i]); | 4687 | __free_page(spd.pages[i]); |
4688 | break; | 4688 | break; |
4689 | } | 4689 | } |
4690 | spd.partial[i].offset = 0; | 4690 | spd.partial[i].offset = 0; |
4691 | spd.partial[i].len = iter->seq.seq.len; | 4691 | spd.partial[i].len = trace_seq_used(&iter->seq); |
4692 | 4692 | ||
4693 | trace_seq_init(&iter->seq); | 4693 | trace_seq_init(&iter->seq); |
4694 | } | 4694 | } |
@@ -5689,7 +5689,8 @@ tracing_stats_read(struct file *filp, char __user *ubuf, | |||
5689 | cnt = ring_buffer_read_events_cpu(trace_buf->buffer, cpu); | 5689 | cnt = ring_buffer_read_events_cpu(trace_buf->buffer, cpu); |
5690 | trace_seq_printf(s, "read events: %ld\n", cnt); | 5690 | trace_seq_printf(s, "read events: %ld\n", cnt); |
5691 | 5691 | ||
5692 | count = simple_read_from_buffer(ubuf, count, ppos, s->buffer, s->seq.len); | 5692 | count = simple_read_from_buffer(ubuf, count, ppos, |
5693 | s->buffer, trace_seq_used(s)); | ||
5693 | 5694 | ||
5694 | kfree(s); | 5695 | kfree(s); |
5695 | 5696 | ||
diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c index 4d0067dd7f88..935cbea78532 100644 --- a/kernel/trace/trace_events.c +++ b/kernel/trace/trace_events.c | |||
@@ -1044,7 +1044,8 @@ event_filter_read(struct file *filp, char __user *ubuf, size_t cnt, | |||
1044 | mutex_unlock(&event_mutex); | 1044 | mutex_unlock(&event_mutex); |
1045 | 1045 | ||
1046 | if (file) | 1046 | if (file) |
1047 | r = simple_read_from_buffer(ubuf, cnt, ppos, s->buffer, s->seq.len); | 1047 | r = simple_read_from_buffer(ubuf, cnt, ppos, |
1048 | s->buffer, trace_seq_used(s)); | ||
1048 | 1049 | ||
1049 | kfree(s); | 1050 | kfree(s); |
1050 | 1051 | ||
@@ -1210,7 +1211,8 @@ subsystem_filter_read(struct file *filp, char __user *ubuf, size_t cnt, | |||
1210 | trace_seq_init(s); | 1211 | trace_seq_init(s); |
1211 | 1212 | ||
1212 | print_subsystem_event_filter(system, s); | 1213 | print_subsystem_event_filter(system, s); |
1213 | r = simple_read_from_buffer(ubuf, cnt, ppos, s->buffer, s->seq.len); | 1214 | r = simple_read_from_buffer(ubuf, cnt, ppos, |
1215 | s->buffer, trace_seq_used(s)); | ||
1214 | 1216 | ||
1215 | kfree(s); | 1217 | kfree(s); |
1216 | 1218 | ||
@@ -1265,7 +1267,8 @@ show_header(struct file *filp, char __user *ubuf, size_t cnt, loff_t *ppos) | |||
1265 | trace_seq_init(s); | 1267 | trace_seq_init(s); |
1266 | 1268 | ||
1267 | func(s); | 1269 | func(s); |
1268 | r = simple_read_from_buffer(ubuf, cnt, ppos, s->buffer, s->seq.len); | 1270 | r = simple_read_from_buffer(ubuf, cnt, ppos, |
1271 | s->buffer, trace_seq_used(s)); | ||
1269 | 1272 | ||
1270 | kfree(s); | 1273 | kfree(s); |
1271 | 1274 | ||
diff --git a/kernel/trace/trace_functions_graph.c b/kernel/trace/trace_functions_graph.c index 6d1342ae7a44..ec35468349a7 100644 --- a/kernel/trace/trace_functions_graph.c +++ b/kernel/trace/trace_functions_graph.c | |||
@@ -1153,6 +1153,9 @@ print_graph_comment(struct trace_seq *s, struct trace_entry *ent, | |||
1153 | return ret; | 1153 | return ret; |
1154 | } | 1154 | } |
1155 | 1155 | ||
1156 | if (trace_seq_has_overflowed(s)) | ||
1157 | goto out; | ||
1158 | |||
1156 | /* Strip ending newline */ | 1159 | /* Strip ending newline */ |
1157 | if (s->buffer[s->seq.len - 1] == '\n') { | 1160 | if (s->buffer[s->seq.len - 1] == '\n') { |
1158 | s->buffer[s->seq.len - 1] = '\0'; | 1161 | s->buffer[s->seq.len - 1] = '\0'; |
@@ -1160,7 +1163,7 @@ print_graph_comment(struct trace_seq *s, struct trace_entry *ent, | |||
1160 | } | 1163 | } |
1161 | 1164 | ||
1162 | trace_seq_puts(s, " */\n"); | 1165 | trace_seq_puts(s, " */\n"); |
1163 | 1166 | out: | |
1164 | return trace_handle_return(s); | 1167 | return trace_handle_return(s); |
1165 | } | 1168 | } |
1166 | 1169 | ||
diff --git a/kernel/trace/trace_seq.c b/kernel/trace/trace_seq.c index 74cacc930c24..f8b45d8792f9 100644 --- a/kernel/trace/trace_seq.c +++ b/kernel/trace/trace_seq.c | |||
@@ -30,7 +30,7 @@ | |||
30 | #define TRACE_SEQ_BUF_LEFT(s) seq_buf_buffer_left(&(s)->seq) | 30 | #define TRACE_SEQ_BUF_LEFT(s) seq_buf_buffer_left(&(s)->seq) |
31 | 31 | ||
32 | /* How much buffer is written? */ | 32 | /* How much buffer is written? */ |
33 | #define TRACE_SEQ_BUF_USED(s) min((s)->seq.len, (unsigned int)(PAGE_SIZE - 1)) | 33 | #define TRACE_SEQ_BUF_USED(s) seq_buf_used(&(s)->seq) |
34 | 34 | ||
35 | /* | 35 | /* |
36 | * trace_seq should work with being initialized with 0s. | 36 | * trace_seq should work with being initialized with 0s. |