linux-kernel - Re: [PATCH] trace-cmd: Don't try to read unmapped memory.

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <4C3F2CDD.80807@caviumnetworks.com>
Date:	Thu, 15 Jul 2010 08:44:29 -0700
From:	David Daney <ddaney@...iumnetworks.com>
To:	rostedt@...dmis.org
CC:	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] trace-cmd: Don't try to read unmapped memory.

This one isn't correct.  It prevents the crash, but also can cause many 
trace records to be omitted.  I will try to come up with a new patch for 
the problem I see.

David Daney


On 07/14/2010 03:12 PM, David Daney wrote:
> When tracecmd_peek_data() reads the last event by calling
> translate_data(), it will get type_len = 0, length = -4.  It had to
> read 8 bytes of data, but it adjusts the index by -4, for a net
> increment of 4.  The invariant that the index points to an entire
> event ceases to hold.
>
> If the index were already at the last event in the mapped area, a
> subsequent tracecmd_peek_data() checks that the index is not beyond
> the end of the mapping (which it isn't), but it assumes that the
> entire event will fit (which it doesn't).  It then attempts to read an
> entire event (8 bytes), but the last 4 bytes are now beyond the end of
> the mapping causing a fault.
>
> My fix is to keep the index pointing at the last record when the
> negative length is encountered.
>
> On my x86_64 workstation, the mappings of the trace data were always
> contiguous with other mapped memory, so the reading of 4 bytes past the
> end of the mapping always fell on another piece of mapped memory, so
> no fault was produced.  Running under valgrind or on a MIPS64 host was
> necessary to produce the fault.
>
> Signed-off-by: David Daney<ddaney@...iumnetworks.com>
> ---
>   trace-input.c |   12 ++++++++++++
>   1 files changed, 12 insertions(+), 0 deletions(-)
>
> diff --git a/trace-input.c b/trace-input.c
> index 398d0f9..2ba346d 100644
> --- a/trace-input.c
> +++ b/trace-input.c
> @@ -1530,6 +1530,18 @@ read_again:
>
>   	type_len = translate_data(handle,&ptr,&extend,&length);
>
> +	if (length<  0) {
> +		/*
> +		 * Negative length indicates the end.  Back up ptr so
> +		 * subsequent reads don't fall off the end of the
> +		 * mapping.
> +		 */
> +		ptr -= 8;
> +		handle->cpu_data[cpu].index = calc_index(handle, ptr, cpu);
> +		handle->cpu_data[cpu].next = NULL;
> +		return NULL;
> +	}
> +
>   	switch (type_len) {
>   	case RINGBUF_TYPE_PADDING:
>   		if (!extend) {

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/