Messages in this thread Patch in this message | | | Date | Tue, 12 Dec 2023 09:33:11 -0500 | Subject | Re: [PATCH v2] tracing: Allow for max buffer data size trace_marker writes | From | Mathieu Desnoyers <> |
| |
On 2023-12-12 09:00, Steven Rostedt wrote: [...] > --- a/kernel/trace/trace.c > +++ b/kernel/trace/trace.c > @@ -7272,6 +7272,7 @@ tracing_mark_write(struct file *filp, const char __user *ubuf, > enum event_trigger_type tt = ETT_NONE; > struct trace_buffer *buffer; > struct print_entry *entry; > + int meta_size; > ssize_t written; > int size; > int len; > @@ -7286,12 +7287,9 @@ tracing_mark_write(struct file *filp, const char __user *ubuf, > if (!(tr->trace_flags & TRACE_ITER_MARKERS)) > return -EINVAL; > > - if (cnt > TRACE_BUF_SIZE) > - cnt = TRACE_BUF_SIZE;
You're removing an early bound check for a size_t userspace input...
> - > - BUILD_BUG_ON(TRACE_BUF_SIZE >= PAGE_SIZE); > - > - size = sizeof(*entry) + cnt + 2; /* add '\0' and possible '\n' */ > + meta_size = sizeof(*entry) + 2; /* add '\0' and possible '\n' */ > + again: > + size = cnt + meta_size;
... and then implicitly casting it into a "int" size variable, which can therefore become a negative value.
Just for the sake of not having to rely on ring_buffer_lock_reserve catching (length > BUF_MAX_DATA_SIZE), I would recommend to add an early check for negative here.
> > /* If less than "<faulted>", then make sure we can still add that */ > if (cnt < FAULTED_SIZE) > @@ -7300,9 +7298,25 @@ tracing_mark_write(struct file *filp, const char __user *ubuf, > buffer = tr->array_buffer.buffer; > event = __trace_buffer_lock_reserve(buffer, TRACE_PRINT, size, > tracing_gen_ctx()); > - if (unlikely(!event)) > + if (unlikely(!event)) { > + /* > + * If the size was greated than what was allowed, then
greater ?
Thanks,
Mathieu
-- Mathieu Desnoyers EfficiOS Inc. https://www.efficios.com
| |