linux-kernel - Re: [PATCH] tracing/lockdep: turn lock->name into an array

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20090413224219.GK5977@nowhere>
Date:	Tue, 14 Apr 2009 00:42:21 +0200
From:	Frederic Weisbecker <fweisbec@...il.com>
To:	Ingo Molnar <mingo@...e.hu>
Cc:	Steven Rostedt <rostedt@...dmis.org>,
	Zhaolei <zhaolei@...fujitsu.com>,
	Tom Zanussi <tzanussi@...il.com>,
	Li Zefan <lizf@...fujitsu.com>,
	LKML <linux-kernel@...r.kernel.org>,
	Peter Zijlstra <peterz@...radead.org>
Subject: Re: [PATCH] tracing/lockdep: turn lock->name into an array

On Tue, Apr 14, 2009 at 12:36:06AM +0200, Frederic Weisbecker wrote:
> Impact: allow filtering by lock name / fix module tracing
> 
> Currently, the "lock acquired" event is traced using a TRACE_EVENT.
> But we can't use the char * type for the name without risking to
> dereference a freed pointer. A lock name can come from a module
> towards lockdep and it is risky to only store its address because we
> defer its name printing.
> 
> That's why this patch uses a fixed array size and copy the name.
> Also it lets us filter the lock name because the event filtering
> doesn't handle the char pointers. Such support is not needed yet since
> the events don't use it for now because it is rarely easy to keep track
> of a string while we defer its output.
> 
> Signed-off-by: Frederic Weisbecker <fweisbec@...il.com>
> ---
>  include/trace/lockdep_event_types.h |   13 +++++++++++--
>  1 files changed, 11 insertions(+), 2 deletions(-)
> 
> diff --git a/include/trace/lockdep_event_types.h b/include/trace/lockdep_event_types.h
> index 863f1e4..68f84f4 100644
> --- a/include/trace/lockdep_event_types.h
> +++ b/include/trace/lockdep_event_types.h
> @@ -32,18 +32,27 @@ TRACE_FORMAT(lock_contended,
>  	TP_FMT("%s", lock->name)
>  	);
>  
> +#define LOCK_NAME_SIZE	25



This constant may look a bit weird.
I just started with the assumption that a full lock name
will rarely exceed this length.

If you agree with it, I will expand the conversion of lockdep
TRACE_FORMAT to TRACE_EVENTS with the same assumption.
So that we will be able to use filters with locks events.

Thanks.



> +
> +/*
> + * We might loose the name if it comes from a module
> + * so we keep a fixed array size of 25, enough for most
> + * usecases.
> + */
> +
>  TRACE_EVENT(lock_acquired,
>  	TP_PROTO(struct lockdep_map *lock, unsigned long ip, s64 waittime),
>  
>  	TP_ARGS(lock, ip, waittime),
>  
>  	TP_STRUCT__entry(
> -		__field(const char *, name)
> +		__array(char, name, LOCK_NAME_SIZE)
>  		__field(unsigned long, wait_usec)
>  		__field(unsigned long, wait_nsec_rem)
>  	),
>  	TP_fast_assign(
> -		__entry->name = lock->name;
> +		strncpy(__entry->name, lock->name, LOCK_NAME_SIZE - 1);
> +		__entry->name[LOCK_NAME_SIZE - 1] = 0;
>  		__entry->wait_nsec_rem = do_div(waittime, NSEC_PER_USEC);
>  		__entry->wait_usec = (unsigned long) waittime;
>  	),
> -- 
> 1.6.1
> 

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