linux-kernel - Re: [tip:tracing/urgent] tracing: Fix trace

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 for Android: free password hash cracker in your pocket

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

Date:	Sun, 06 Dec 2009 10:24:46 +0100
From:	Carsten Emde <Carsten.Emde@...dl.org>
To:	Olof Johansson <olof@...om.net>
CC:	mingo@...hat.com, hpa@...or.com, linux-kernel@...r.kernel.org,
	rostedt@...dmis.org, tglx@...utronix.de,
	linux-tip-commits@...r.kernel.org
Subject: Re: [tip:tracing/urgent] tracing: Fix trace_marker output

Olof,

> On Sun, Nov 22, 2009 at 10:24:48AM +0000, tip-bot for Carsten Emde wrote:
>> Commit-ID:  c13d2f7c3231e873f30db92b96c8caa48f100f33
>> Gitweb:     http://git.kernel.org/tip/c13d2f7c3231e873f30db92b96c8caa48f100f33
>> Author:     Carsten Emde<Carsten.Emde@...dl.org>
>> AuthorDate: Mon, 16 Nov 2009 20:56:13 +0100
>> Committer:  Steven Rostedt<rostedt@...dmis.org>
>> CommitDate: Tue, 17 Nov 2009 09:19:06 -0500
>> tracing: Fix trace_marker output
>> When a string was written to<debugfs>/tracing/trace_marker, some
>> strange characters appeared in the trace output instead of the
>> string, since a vprint function erroneously called a vararg print
>> function with a va_list argument. This patch fixes the problem and
>> simplifies the related code.
>
> [...]
>
>> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
>> index 03c7fd5..12b49ca 100644
>> --- a/kernel/trace/trace.c
>> +++ b/kernel/trace/trace.c
>> @@ -1361,10 +1361,11 @@ int trace_array_vprintk(struct trace_array *tr,
>>   	pause_graph_tracing();
>>   	raw_local_irq_save(irq_flags);
>>   	__raw_spin_lock(&trace_buf_lock);
>> -	len = vsnprintf(trace_buf, TRACE_BUF_SIZE, fmt, args);
>> -
>> -	len = min(len, TRACE_BUF_SIZE-1);
>> -	trace_buf[len] = 0;
>> +	if (args == NULL) {
>> +		strncpy(trace_buf, fmt, TRACE_BUF_SIZE);
>> +		len = strlen(trace_buf);
>> +	} else
>> +		len = vsnprintf(trace_buf, TRACE_BUF_SIZE, fmt, args);
> Comparing a va_list with NULL is bogus. It's supposed to be treated like
> an opaque type and only be manipulated with va_* accessors.
>
> I wouldn't really care much, but it broke builds on some ARM platforms:
>
> kernel/trace/trace.c: In function 'trace_array_vprintk':
> kernel/trace/trace.c:1364: error: invalid operands to binary == (have 'va_list' and 'void *')
> kernel/trace/trace.c: In function 'tracing_mark_write':
> kernel/trace/trace.c:3349: error: incompatible type for argument 3 of 'trace_vprintk'
Oops, sorry. Didn't know that the gcc warn/error level is so much 
different across platforms.

>[..]
> Oh, and I haven't had a chance to actually test and make sure it does
> what is expected, since I don't have a testcase for it.
The testcase is:
# cd /sys/kernel/debug/tracing/
[root@...iv1 tracing]# echo "I am a marker" >trace_marker
[root@...iv1 tracing]# tail -1 trace
             bash-6347  [002] 303160.793532: 0: I am a marker

> +	ret = trace_vprintk(ip, "%s", ap);
This prevents the '%' character to be used in an output string. If used, 
it may horribly crash the kernel.

I'll investigate the situation further and try to come back with a 
better solution.

	Carsten.
--
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/