From: [email protected]
> Sent: 30 August 2018 03:20
>
> Some implementations of libc do not support the 'm' width modifier
> as part of the scanf string format specifier. This can cause the
> parsing to fail. Since the parser never checks if the scanf
> parsing was successesful, this can result in a crash.
>
> Change the comm string to be allocated as a fixed size instead of
> dynamically using 'm' scanf width modifier. This can be safely done
> since comm size is limited to 16 bytes by TASK_COMM_LEN within the
> kernel.
>
> This change prevents perf from crashing when linked against bionic
> as well as reduces the total number of heap allocations and frees
> invoked while accomplishing the same task.
>
> Signed-off-by: Chris Phlipot <[email protected]>
> ---
> tools/perf/util/trace-event-parse.c | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/tools/perf/util/trace-event-parse.c
> b/tools/perf/util/trace-event-parse.c
> index 920b1d58a068..e76214f8d596 100644
> --- a/tools/perf/util/trace-event-parse.c
> +++ b/tools/perf/util/trace-event-parse.c
> @@ -164,16 +164,15 @@ void parse_ftrace_printk(struct tep_handle *pevent,
> void parse_saved_cmdline(struct tep_handle *pevent,
> char *file, unsigned int size __maybe_unused)
> {
> - char *comm;
> + char comm[17]; /* Max comm length in the kernel is 16. */
> char *line;
> char *next = NULL;
> int pid;
>
> line = strtok_r(file, "\n", &next);
> while (line) {
> - sscanf(line, "%d %ms", &pid, &comm);
> - tep_register_comm(pevent, comm, pid);
> - free(comm);
> + if (sscanf(line, "%d %16s", &pid, comm) == 2)
> + tep_register_comm(pevent, comm, pid);
> line = strtok_r(NULL, "\n", &next);
Seems to me that sscanf is the wrong tool for the job (as usual).
Why not just:
pid = strtoul(line, &comm, 10);
while (*comm == ' ')
comm++;
tep_register_comm(pevent, comm, pid);
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT,
UK
Registration No: 1397386 (Wales)
