Commit 9178412ddf5a ("tracing: probeevent: Return consumed
bytes of dynamic area") improved the string fetching
mechanism by returning the number of required bytes after
copying the argument to the dynamic area. However, this
return value is now only used to increment the pointer
inside the dynamic area but misses updating the 'maxlen'
variable which indicates the remaining space in the dynamic
area.
This means that fetch_store_string() always reads the *total*
size of the dynamic area from the data_loc pointer instead of
the *remaining* size (and passes it along to
strncpy_from_{user,unsafe}) even if we're already about to
copy data into the middle of the dynamic area.
Fixes: 9178412ddf5a ("tracing: probeevent: Return consumed bytes of dynamic area")
Signed-off-by: Andreas Ziegler <[email protected]>
---
kernel/trace/trace_probe_tmpl.h | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h
index 5c56afc17cf8..0cf953e47584 100644
--- a/kernel/trace/trace_probe_tmpl.h
+++ b/kernel/trace/trace_probe_tmpl.h
@@ -182,8 +182,10 @@ store_trace_args(void *data, struct trace_probe *tp, struct pt_regs *regs,
ret = process_fetch_insn(arg->code, regs, dl, base);
if (unlikely(ret < 0 && arg->dynamic))
*dl = make_data_loc(0, dyndata - base);
- else
+ else {
dyndata += ret;
+ maxlen -= ret;
+ }
}
}
--
2.17.1
On Tue, 22 Jan 2019 13:48:48 +0100
Andreas Ziegler <[email protected]> wrote:
> Commit 9178412ddf5a ("tracing: probeevent: Return consumed
> bytes of dynamic area") improved the string fetching
> mechanism by returning the number of required bytes after
> copying the argument to the dynamic area. However, this
> return value is now only used to increment the pointer
> inside the dynamic area but misses updating the 'maxlen'
> variable which indicates the remaining space in the dynamic
> area.
Oops! Good catch! :)
>
> This means that fetch_store_string() always reads the *total*
> size of the dynamic area from the data_loc pointer instead of
> the *remaining* size (and passes it along to
> strncpy_from_{user,unsafe}) even if we're already about to
> copy data into the middle of the dynamic area.
>
This looks good to me.
Acked-by: Masami Hiramatsu <[email protected]>
Thank you!!
> Fixes: 9178412ddf5a ("tracing: probeevent: Return consumed bytes of dynamic area")
> Signed-off-by: Andreas Ziegler <[email protected]>
> ---
> kernel/trace/trace_probe_tmpl.h | 4 +++-
> 1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h
> index 5c56afc17cf8..0cf953e47584 100644
> --- a/kernel/trace/trace_probe_tmpl.h
> +++ b/kernel/trace/trace_probe_tmpl.h
> @@ -182,8 +182,10 @@ store_trace_args(void *data, struct trace_probe *tp, struct pt_regs *regs,
> ret = process_fetch_insn(arg->code, regs, dl, base);
> if (unlikely(ret < 0 && arg->dynamic))
> *dl = make_data_loc(0, dyndata - base);
> - else
> + else {
> dyndata += ret;
> + maxlen -= ret;
> + }
> }
> }
>
> --
> 2.17.1
>
--
Masami Hiramatsu <[email protected]>
Commit 9178412ddf5a ("tracing: probeevent: Return consumed
bytes of dynamic area") improved the string fetching
mechanism by returning the number of required bytes after
copying the argument to the dynamic area. However, this
return value is now only used to increment the pointer
inside the dynamic area but misses updating the 'maxlen'
variable which indicates the remaining space in the dynamic
area.
This means that fetch_store_string() always reads the *total*
size of the dynamic area from the data_loc pointer instead of
the *remaining* size (and passes it along to
strncpy_from_{user,unsafe}) even if we're already about to
copy data into the middle of the dynamic area.
Fixes: 9178412ddf5a ("tracing: probeevent: Return consumed bytes of dynamic area")
Signed-off-by: Andreas Ziegler <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
---
v2: follow coding guidelines for braces
kernel/trace/trace_probe_tmpl.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h
index 5c56afc17cf8..4737bb8c07a3 100644
--- a/kernel/trace/trace_probe_tmpl.h
+++ b/kernel/trace/trace_probe_tmpl.h
@@ -180,10 +180,12 @@ store_trace_args(void *data, struct trace_probe *tp, struct pt_regs *regs,
if (unlikely(arg->dynamic))
*dl = make_data_loc(maxlen, dyndata - base);
ret = process_fetch_insn(arg->code, regs, dl, base);
- if (unlikely(ret < 0 && arg->dynamic))
+ if (unlikely(ret < 0 && arg->dynamic)) {
*dl = make_data_loc(0, dyndata - base);
- else
+ } else {
dyndata += ret;
+ maxlen -= ret;
+ }
}
}
--
2.17.1
Commit 9178412ddf5a ("tracing: probeevent: Return consumed
bytes of dynamic area") improved the string fetching
mechanism by returning the number of required bytes after
copying the argument to the dynamic area. However, this
return value is now only used to increment the pointer
inside the dynamic area but misses updating the 'maxlen'
variable which indicates the remaining space in the dynamic
area.
This means that fetch_store_string() always reads the *total*
size of the dynamic area from the data_loc pointer instead of
the *remaining* size (and passes it along to
strncpy_from_{user,unsafe}) even if we're already about to
copy data into the middle of the dynamic area.
Fixes: 9178412ddf5a ("tracing: probeevent: Return consumed bytes of dynamic area")
Signed-off-by: Andreas Ziegler <[email protected]>
Acked-by: Masami Hiramatsu <[email protected]>
---
v2: follow coding guidelines for braces
kernel/trace/trace_probe_tmpl.h | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/kernel/trace/trace_probe_tmpl.h b/kernel/trace/trace_probe_tmpl.h
index 5c56afc17cf8..4737bb8c07a3 100644
--- a/kernel/trace/trace_probe_tmpl.h
+++ b/kernel/trace/trace_probe_tmpl.h
@@ -180,10 +180,12 @@ store_trace_args(void *data, struct trace_probe *tp, struct pt_regs *regs,
if (unlikely(arg->dynamic))
*dl = make_data_loc(maxlen, dyndata - base);
ret = process_fetch_insn(arg->code, regs, dl, base);
- if (unlikely(ret < 0 && arg->dynamic))
+ if (unlikely(ret < 0 && arg->dynamic)) {
*dl = make_data_loc(0, dyndata - base);
- else
+ } else {
dyndata += ret;
+ maxlen -= ret;
+ }
}
}
--
2.17.1
On Wed, 6 Feb 2019 20:00:13 +0100
Andreas Ziegler <[email protected]> wrote:
> Commit 9178412ddf5a ("tracing: probeevent: Return consumed
> bytes of dynamic area") improved the string fetching
> mechanism by returning the number of required bytes after
> copying the argument to the dynamic area. However, this
> return value is now only used to increment the pointer
> inside the dynamic area but misses updating the 'maxlen'
> variable which indicates the remaining space in the dynamic
> area.
>
> This means that fetch_store_string() always reads the *total*
> size of the dynamic area from the data_loc pointer instead of
> the *remaining* size (and passes it along to
> strncpy_from_{user,unsafe}) even if we're already about to
> copy data into the middle of the dynamic area.
>
> Fixes: 9178412ddf5a ("tracing: probeevent: Return consumed bytes of dynamic area")
> Signed-off-by: Andreas Ziegler <[email protected]>
> Acked-by: Masami Hiramatsu <[email protected]>
> ---
>
I applied it and will start testing it. But just an FYI, please send
new versions of a patch as a separate thread. Sending it as a reply to
is likely to have it get missed, as maintainers usually search their
inboxes threaded, and only look at patches that are the start of a
thread. I just happened to have this one marked to look at.
-- Steve