linux-kernel - Re: [PATCH v2 2/7] perf/core: Use ioctl to communicate driver configuration to kernel

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:   Fri, 6 Jul 2018 18:22:22 -0500
From:   Kim Phillips <kim.phillips@....com>
To:     Mathieu Poirier <mathieu.poirier@...aro.org>
Cc:     peterz@...radead.org, acme@...nel.org, mark.rutland@....com,
        linux-s390@...r.kernel.org, alexander.shishkin@...ux.intel.com,
        will.deacon@....com, hpa@...or.com, heiko.carstens@...ibm.com,
        adrian.hunter@...el.com, ast@...nel.org, mingo@...hat.com,
        linux-arm-kernel@...ts.infradead.org, gregkh@...uxfoundation.org,
        schwidefsky@...ibm.com, namhyung@...nel.org, tglx@...utronix.de,
        jolsa@...hat.com, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/7] perf/core: Use ioctl to communicate driver
 configuration to kernel

On Thu,  5 Jul 2018 16:13:42 -0600
Mathieu Poirier <mathieu.poirier@...aro.org> wrote:

Hi Mathieu,

> This patch adds the mechanic needed for user space to send PMU specific
                      ^^^^^^^^
I think you meant 'mechanism' here:  mechanics fix cars :)

> +static void perf_drv_config_replace(struct perf_event *event, void *drv_data)
> +{
> +	unsigned long flags;
> +	void *old_drv_data;
> +	struct pmu_drv_config *drv_config = &event->hw.drv_config;
> +
> +	if (!has_drv_config(event))
> +		return;
> +
> +	/* Children take their configuration from their parent */
> +	if (event->parent)
> +		return;
> +
> +	/* Make sure the PMU doesn't get a handle on the data */
> +	raw_spin_lock_irqsave(&drv_config->lock, flags);
> +
> +	old_drv_data = drv_config->config;
> +	drv_config->config = drv_data;
> +
> +	raw_spin_unlock_irqrestore(&drv_config->lock, flags);
> +
> +	/* Free PMU private data allocated by pmu::drv_config_validate() */
> +	event->pmu->drv_config_free(old_drv_data);
> +}

I got this stacktrace whilst testing a perf tool *without* this series
applied, running on a kernel *with* this series applied:

[  132.942054] INFO: trying to register non-static key.
[  132.946964] the code is fine but needs lockdep annotation.
[  132.952389] turning off the locking correctness validator.
[  132.957818] CPU: 2 PID: 2835 Comm: perf64-sans Not tainted 4.18.0-rc3-00196-g5b5d957532a8-dirty #146
[  132.966856] Hardware name: ARM LTD ARM Juno Development Platform/ARM Juno Development Platform, BIOS EDK II Jan 23 2017
[  132.977527] Call trace:
[  132.979947]  dump_backtrace+0x0/0x1c0
[  132.983567]  show_stack+0x24/0x30
[  132.986845]  dump_stack+0x90/0xb4
[  132.990122]  register_lock_class+0x57c/0x580
[  132.994343]  __lock_acquire.isra.12+0x6c/0x980
[  132.998736]  lock_acquire+0x100/0x1e8
[  133.002357]  _raw_spin_lock_irqsave+0x58/0x78
[  133.006667]  perf_drv_config_replace+0x4c/0x80
[  133.011061]  _free_event+0xbc/0x460
[  133.014507]  put_event+0x2c/0x38
[  133.017697]  perf_event_release_kernel+0x1ac/0x300
[  133.022434]  perf_release+0x10/0x20
[  133.025883]  __fput+0xa8/0x1e0
[  133.028901]  ____fput+0x20/0x30
[  133.032006]  task_work_run+0xa0/0xd0
[  133.035539]  do_notify_resume+0x118/0x120
[  133.039503]  work_pending+0x8/0x10

Is a raw_spin_lock_init missing perhaps?

Thanks,

Kim