linux-kernel - Re: [PATCH] Perf: bug fix: distinguish between rename and exec

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: <CAA25o9QLbUiTWr6moqYxVkcvKrOCGO9qWyN6HVzo_RWXhqUq+Q@mail.gmail.com>
Date:	Wed, 15 Feb 2012 09:22:50 -0800
From:	Luigi Semenzato <semenzato@...omium.org>
To:	David Ahern <dsahern@...il.com>
Cc:	Peter Zijlstra <a.p.zijlstra@...llo.nl>,
	Alexander Viro <viro@...iv.linux.org.uk>,
	Paul Mackerras <paulus@...ba.org>, Ingo Molnar <mingo@...e.hu>,
	Arnaldo Carvalho de Melo <acme@...stprotocols.net>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Vasiliy Kulikov <segoon@...nwall.com>,
	Stephen Wilson <wilsons@...rt.ca>,
	Oleg Nesterov <oleg@...hat.com>, Tejun Heo <tj@...nel.org>,
	Paul Gortmaker <paul.gortmaker@...driver.com>,
	Andi Kleen <ak@...ux.intel.com>,
	Lucas De Marchi <lucas.demarchi@...fusion.mobi>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	"Eric W. Biederman" <ebiederm@...ssion.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>,
	Frederic Weisbecker <fweisbec@...il.com>,
	Namhyung Kim <namhyung@...il.com>,
	Robert Richter <robert.richter@....com>,
	linux-kernel@...r.kernel.org, sonnyrao@...omium.org,
	olofj@...omium.org, eranian@...gle.com
Subject: Re: [PATCH] Perf: bug fix: distinguish between rename and exec

On Wed, Feb 15, 2012 at 8:57 AM, David Ahern <dsahern@...il.com> wrote:
> On 2/15/12 5:48 AM, Peter Zijlstra wrote:
>>
>> On Mon, 2012-02-13 at 20:56 -0800, Luigi Semenzato wrote:
>>>
>>> This makes it possible for "perf report" to distinguish between an exec
>>> and
>>> a rename (for instance from prctl(PR_SET_NAME)).  Currently a similar
>>> COMM
>>> record is produced for the two events.  Perf report assumes all COMM
>>> records
>>> are execs and discards the old mappings.  Without mappings, perf report
>>> can no longer correlate sampled IPs to the functions containing them,
>>> and collapses all samples into a single bucket.
>>>
>>> This change maintains backward compatibility in both directions, i.e. old
>>> version of perf will continue to work on new perf.data files in the same
>>> way, and new versions of perf on old data files.
>>>
>>> Another solution would be to not send COMM records for renames.  Although
>>> it seems reasonable, it might break existing setups.
>>
>>
>> Uhm, didn't you argue its already broken?
>>
>>> +++ b/fs/exec.c
>>> @@ -1052,7 +1052,7 @@ char *get_task_comm(char *buf, struct task_struct
>>> *tsk)
>>>  }
>>>  EXPORT_SYMBOL_GPL(get_task_comm);
>>>
>>> -void set_task_comm(struct task_struct *tsk, char *buf)
>>> +void set_task_comm(struct task_struct *tsk, char *buf, bool is_rename)
>>>  {
>>>        task_lock(tsk);
>>>
>>> @@ -1068,7 +1068,7 @@ void set_task_comm(struct task_struct *tsk, char
>>> *buf)
>>>        wmb();
>>>        strlcpy(tsk->comm, buf, sizeof(tsk->comm));
>>>        task_unlock(tsk);
>>> -       perf_event_comm(tsk);
>>> +       perf_event_comm(tsk, is_rename);
>>>  }
>>
>>
>> I really dislike changing generic code purely for the purpose of
>> instrumentation like this. Better to pull perf_event_comm() out of here
>> if you want to change semantics.
>>
>> Personally I couldn't care less about renames, I think they're a waste
>> of time, so I'm ok with the simple patch moving the perf_event_comm()
>> into setup_new_exec() and possibly renaming it to perf_event_exec().
>>
>> Acme, do you care about renames?
>>
>
> I'm not Acme, but I do care. We use a lot of processes with named threads
> that give users an idea about the function of a particular thread.
>
> I do not understand the use case targeted by the patch -- what kind of
> processes run for some amount of time and then decide to change task names?

Any process that makes a prctl(PR_SET_NAME) call loses its mappings,
no matter when makes the call.  The perf records for that process look
like this:

COMM (for the initial exec)
MMAP (the executable)
MMAP (1st dll)
MMAP (2nd dll)
...
COMM (for the prctl)

The second COMM flushes the old mappings, and all samples from then on
cannot be classified.  This is easily reproducible with a small
program, which I would be happy to send.

I found this while trying to use perf with Chrome on Chrome OS.
Chrome forks and execs all the time, and calls prctl() in the thread
library.
--
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/