Re: [PATCH] trace-cmd: Fix segmentation fault in tracecmd_read_at() in specific use case

[Steven Rostedt <rostedt@goodmis.org>]

On Fri, 11 Oct 2019 14:33:53 +0300
"Tzvetomir Stoyanov (VMware)" <tz.stoyanov@gmail.com> wrote:

> There is a segmentation fault in update_page_info() when the requested page
> is not loaded, handle->cpu_data[cpu].page is NULL. The problematic flow starts
> from tracecmd_read_at() API, when reading offset in the first page (less than 4K),
> and this page is still not loaded yet. The problem can be observed randomly -
> there is a sporadic KernelShark crash when loading a file, browsing and
> zooming events.
> 
> https://bugzilla.kernel.org/show_bug.cgi?id=205165
> Signed-off-by: Tzvetomir Stoyanov (VMware) <tz.stoyanov@gmail.com>
> ---
>  lib/trace-cmd/trace-input.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/lib/trace-cmd/trace-input.c b/lib/trace-cmd/trace-input.c
> index 6102eb3..da77418 100644
> --- a/lib/trace-cmd/trace-input.c
> +++ b/lib/trace-cmd/trace-input.c
> @@ -1278,7 +1278,8 @@ tracecmd_read_at(struct tracecmd_input *handle, unsigned long long offset,
>  	/* check to see if we have this page already */
>  	for (cpu = 0; cpu < handle->cpus; cpu++) {
>  		if (handle->cpu_data[cpu].offset == page_offset &&
> -		    handle->cpu_data[cpu].file_size)
> +		    handle->cpu_data[cpu].file_size &&
> +		    handle->cpu_data[cpu].page)
>  			break;
>  	}
>  

This does indeed look like a legit bug. But instead of checking here
for page not existing, since it's not part of the criteria for finding
the page (if the offset matches, we still want to break), lets do the
check below:

	if (cpu < handle->cpus && handle->cpu_data[cpu].page) {
		if (pcpu)
			*pcpu = cpu;
		return read_event(handle, offset, cpu);
	} else
		return find_and_read_event(handle, offset, pcpu);

-- Steve