2013-09-25 12:07:57

by Knut Petersen

[permalink] [raw]
Subject: [PATCH] perf: Prevent divide by zero exception in kernel/events/core.c

Any value written to /proc/sys/kernel/perf_event_max_sample_rate
is accepted and used as a divisor in perf_proc_update_handler().

Obviously that is wrong. 0 will cause a divide by 0 exception,
and negative values are unreasonable.

The attached patch enforces a lower limit of 1.

cu,
Knut


Attachments:
0001-Enforce-1-as-lower-limit-for-perf_event_max_sample_r.patch (1.10 kB)

2013-09-25 12:13:00

by Peter Zijlstra

[permalink] [raw]
Subject: Re: [PATCH] perf: Prevent divide by zero exception in kernel/events/core.c

On Wed, Sep 25, 2013 at 02:07:35PM +0200, Knut Petersen wrote:
> From: Knut Petersen <[email protected]>
> Date: Wed, 25 Sep 2013 13:34:57 +0200
> Subject: [PATCH] Enforce 1 as lower limit for perf_event_max_sample_rate
>
> /proc/sys/kernel/perf_event_max_sample_rate will accept
> negative values as well as 0.
>
> Negative values are unreasonable, and 0 causes a
> divide by zero exception in perf_proc_update_handler.
>
> This patch enforces a lower limit of 1.
>
> Signed-off-by: Knut Petersen <[email protected]>
> ---
> kernel/events/core.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index dd236b6..350bb53 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -198,6 +198,9 @@ int perf_proc_update_handler(struct ctl_table *table, int write,
> if (ret || !write)
> return ret;
>
> + if (sysctl_perf_event_sample_rate <= 0)
> + sysctl_perf_event_sample_rate = 1;
> +
> max_samples_per_tick = DIV_ROUND_UP(sysctl_perf_event_sample_rate, HZ);
> perf_sample_period_ns = NSEC_PER_SEC / sysctl_perf_event_sample_rate;
> update_perf_cpu_limits();

The 'normal' way of doing that is changing the sysctl table entry to use
proc_dointvec_minmax and set .extra = &one. See for example
&sysctl_sched_cfs_bandwidth_slice in kernel/sysctl.c

2013-09-25 12:46:20

by Knut Petersen

[permalink] [raw]
Subject: Re: [PATCH] perf: Prevent divide by zero exception in kernel/events/core.c

On 25.09.2013 14:12, Peter Zijlstra wrote:
> The 'normal' way of doing that is changing the sysctl table entry to use
> proc_dointvec_minmax and set .extra = &one. See for example
> &sysctl_sched_cfs_bandwidth_slice in kernel/sysctl.c

Thanks. V2 attached.

cu,
Knut


Attachments:
0001-Enforce-1-as-lower-limit-for-perf_event_max_sample_r.patch (1.50 kB)
Subject: [tip:perf/core] perf: Enforce 1 as lower limit for perf_event_max_sample_rate

Commit-ID: 723478c8a471403c53cf144999701f6e0c4bbd11
Gitweb: http://git.kernel.org/tip/723478c8a471403c53cf144999701f6e0c4bbd11
Author: Knut Petersen <[email protected]>
AuthorDate: Wed, 25 Sep 2013 14:29:37 +0200
Committer: Ingo Molnar <[email protected]>
CommitDate: Fri, 4 Oct 2013 10:06:07 +0200

perf: Enforce 1 as lower limit for perf_event_max_sample_rate

/proc/sys/kernel/perf_event_max_sample_rate will accept
negative values as well as 0.

Negative values are unreasonable, and 0 causes a
divide by zero exception in perf_proc_update_handler.

This patch enforces a lower limit of 1.

Signed-off-by: Knut Petersen <[email protected]>
Signed-off-by: Peter Zijlstra <[email protected]>
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Ingo Molnar <[email protected]>
---
kernel/events/core.c | 2 +-
kernel/sysctl.c | 1 +
2 files changed, 2 insertions(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index d49a9d2..b25d65c 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -193,7 +193,7 @@ int perf_proc_update_handler(struct ctl_table *table, int write,
void __user *buffer, size_t *lenp,
loff_t *ppos)
{
- int ret = proc_dointvec(table, write, buffer, lenp, ppos);
+ int ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);

if (ret || !write)
return ret;
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index b2f06f3..2a9db91 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1049,6 +1049,7 @@ static struct ctl_table kern_table[] = {
.maxlen = sizeof(sysctl_perf_event_sample_rate),
.mode = 0644,
.proc_handler = perf_proc_update_handler,
+ .extra1 = &one,
},
{
.procname = "perf_cpu_time_max_percent",