linux-kernel - Re: [GIT PULL] updates for oprofile

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]

Date:	Tue, 27 Apr 2010 19:40:26 +0200
From:	Robert Richter <robert.richter@....com>
To:	Phil Carmody <ext-phil.2.carmody@...ia.com>
CC:	schwidefsky@...ibm.com, mingo@...e.hu,
	linux-kernel@...r.kernel.org,
	oprofile-list <oprofile-list@...ts.sourceforge.net>
Subject: Re: [GIT PULL] updates for oprofile

(cc'ing oprofile-list)

On 27.04.10 18:25:44, Phil Carmody wrote:
> Ingo, et al., 
> 
> Regarding today's pulled request, containing:
> 
> commit bc078e4eab65f11bbaeed380593ab8151b30d703
> Author: Martin Schwidefsky <schwidef...@...ibm.com>
> Date:   Tue Mar 2 16:01:10 2010 +0100
> 
>     oprofile: convert oprofile from timer_hook to hrtimer
>     
> 
> Information is a touch scant, as I'm doing the investigation as I
> write, but I believe that that patch can cause ooops regressions
> via a null-pointer dereference in oprofile_add_sample().
> 
> That function declares:
> """
> /**
>  * Add a sample. This may be called from any context.
>  */
> void oprofile_add_sample(struct pt_regs * const regs, unsigned long event);
> """
> 
> And begins:
> """
> void oprofile_add_sample(struct pt_regs * const regs, unsigned long event)
> {
>         int is_kernel = !user_mode(regs);
> """
> 
> Where on at least two major architectures (Arm, x86), user_mode()
> unconditionally dereferences its parameter.
> 
> Now oprofile_add_sample() is called from this context:
> """
> static enum hrtimer_restart oprofile_hrtimer_notify(struct hrtimer *hrtimer)
> {
>          oprofile_add_sample(get_irq_regs(), 0);
> """
> 
> And get_irq_regs() is NULL when not in an IRQ context.

Perf is simply dropping the sample in such cases, see:

 kernel/perf_event.c:perf_swevent_hrtimer()

So at quick fix would be to check for a null pointer also. But,
according to this:

 http://www.mail-archive.com/linux-omap@vger.kernel.org/msg14074.html

samples will be incorrect then since only interrupt context is
profiled. It seems there is no solution available right now.

-Robert

> 
> Bang.
> 
> An example of this kind of thing kicking in has already been encountered 
> last year:
> http://www.mail-archive.com/linux-omap@vger.kernel.org/msg14069.html
> (That thread got a little side-tracked onto OMAP specifics, but the 
> original report is topical.)
> 
> Now would be a very good time for the "many eyes" principle to kick in.
> 
> I'm now looking into workarounds, but nothing that I'd necessarily
> want to submit as a real fix.
> 
> Phil
> cc:'d replies appreciated
> 

-- 
Advanced Micro Devices, Inc.
Operating System Research Center
email: robert.richter@....com

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