[PATCH 01/14] sysctl: provide callback for write into ctl_table entry

[PATCH 01/14] sysctl: provide callback for write into ctl_table entry

[Sasha Levin]

Provide a callback that will be called when writing to a ctl_table
entry after the user input has been validated.

This will simplify user input checks since now it will be possible to
remove them out of the proc_handler.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 fs/proc/proc_sysctl.c  |    4 ++++
 include/linux/sysctl.h |    1 +
 2 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
index 21d836f..190db28 100644
--- a/fs/proc/proc_sysctl.c
+++ b/fs/proc/proc_sysctl.c
@@ -507,6 +507,10 @@ static ssize_t proc_sys_call_handler(struct file *filp, void __user *buf,
 	error = table->proc_handler(table, write, buf, &res, ppos);
 	if (!error)
 		error = res;
+
+	if (!error && write && table->callback)
+		error = table->callback();
+
 out:
 	sysctl_head_finish(head);
 
diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
index c34b4c8..27c14cf 100644
--- a/include/linux/sysctl.h
+++ b/include/linux/sysctl.h
@@ -1022,6 +1022,7 @@ struct ctl_table
 	struct ctl_table_poll *poll;
 	void *extra1;
 	void *extra2;
+	int (*callback)(void);		/* Called when entry is written to */
 };
 
 struct ctl_node {
-- 
1.7.8.5

[PATCH 02/14] sched debug,sysctl: remove proc input checks out of sysctl handlers

[Sasha Levin]

Simplify sysctl handler by removing user input checks and using the callback
provided by the sysctl table.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 include/linux/sched.h |    4 +---
 kernel/sched/fair.c   |    8 +-------
 kernel/sysctl.c       |   12 ++++++++----
 3 files changed, 10 insertions(+), 14 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 7d2acbd..722da9a 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2136,9 +2136,7 @@ extern unsigned int sysctl_sched_time_avg;
 extern unsigned int sysctl_timer_migration;
 extern unsigned int sysctl_sched_shares_window;
 
-int sched_proc_update_handler(struct ctl_table *table, int write,
-		void __user *buffer, size_t *length,
-		loff_t *ppos);
+int sched_proc_update_handler(void);
 #endif
 #ifdef CONFIG_SCHED_DEBUG
 static inline unsigned int get_sysctl_timer_migration(void)
diff --git a/kernel/sched/fair.c b/kernel/sched/fair.c
index e955364..20ccdc8 100644
--- a/kernel/sched/fair.c
+++ b/kernel/sched/fair.c
@@ -558,16 +558,10 @@ struct sched_entity *__pick_last_entity(struct cfs_rq *cfs_rq)
  * Scheduling class statistics methods:
  */
 
-int sched_proc_update_handler(struct ctl_table *table, int write,
-		void __user *buffer, size_t *lenp,
-		loff_t *ppos)
+int sched_proc_update_handler(void)
 {
-	int ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
 	int factor = get_update_sysctl_factor();
 
-	if (ret || !write)
-		return ret;
-
 	sched_nr_latency = DIV_ROUND_UP(sysctl_sched_latency,
 					sysctl_sched_min_granularity);
 
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index ba133ec..23f1ac6 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -269,7 +269,8 @@ static struct ctl_table kern_table[] = {
 		.data		= &sysctl_sched_min_granularity,
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
-		.proc_handler	= sched_proc_update_handler,
+		.proc_handler	= proc_dointvec_minmax,
+		.callback	= sched_proc_update_handler,
 		.extra1		= &min_sched_granularity_ns,
 		.extra2		= &max_sched_granularity_ns,
 	},
@@ -278,7 +279,8 @@ static struct ctl_table kern_table[] = {
 		.data		= &sysctl_sched_latency,
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
-		.proc_handler	= sched_proc_update_handler,
+		.proc_handler	= proc_dointvec_minmax,
+		.callback	= sched_proc_update_handler,
 		.extra1		= &min_sched_granularity_ns,
 		.extra2		= &max_sched_granularity_ns,
 	},
@@ -287,7 +289,8 @@ static struct ctl_table kern_table[] = {
 		.data		= &sysctl_sched_wakeup_granularity,
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
-		.proc_handler	= sched_proc_update_handler,
+		.proc_handler	= proc_dointvec_minmax,
+		.callback	= sched_proc_update_handler,
 		.extra1		= &min_wakeup_granularity_ns,
 		.extra2		= &max_wakeup_granularity_ns,
 	},
@@ -296,7 +299,8 @@ static struct ctl_table kern_table[] = {
 		.data		= &sysctl_sched_tunable_scaling,
 		.maxlen		= sizeof(enum sched_tunable_scaling),
 		.mode		= 0644,
-		.proc_handler	= sched_proc_update_handler,
+		.proc_handler	= proc_dointvec_minmax,
+		.callback	= sched_proc_update_handler,
 		.extra1		= &min_sched_tunable_scaling,
 		.extra2		= &max_sched_tunable_scaling,
 	},
-- 
1.7.8.5

[PATCH 03/14] sched rt,sysctl: remove proc input checks out of sysctl handlers

[Sasha Levin]

Simplify sysctl handler by removing user input checks and using the callback
provided by the sysctl table.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 include/linux/sched.h |    4 +---
 kernel/sched/core.c   |   25 ++++++++++---------------
 kernel/sysctl.c       |    6 ++++--
 3 files changed, 15 insertions(+), 20 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 722da9a..9509d80 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -2152,9 +2152,7 @@ static inline unsigned int get_sysctl_timer_migration(void)
 extern unsigned int sysctl_sched_rt_period;
 extern int sysctl_sched_rt_runtime;
 
-int sched_rt_handler(struct ctl_table *table, int write,
-		void __user *buffer, size_t *lenp,
-		loff_t *ppos);
+int sched_rt_handler(void);
 
 #ifdef CONFIG_SCHED_AUTOGROUP
 extern unsigned int sysctl_sched_autogroup_enabled;
diff --git a/kernel/sched/core.c b/kernel/sched/core.c
index 477b998..ca4a806 100644
--- a/kernel/sched/core.c
+++ b/kernel/sched/core.c
@@ -7573,9 +7573,7 @@ static int sched_rt_global_constraints(void)
 }
 #endif /* CONFIG_RT_GROUP_SCHED */
 
-int sched_rt_handler(struct ctl_table *table, int write,
-		void __user *buffer, size_t *lenp,
-		loff_t *ppos)
+int sched_rt_handler(void)
 {
 	int ret;
 	int old_period, old_runtime;
@@ -7585,19 +7583,16 @@ int sched_rt_handler(struct ctl_table *table, int write,
 	old_period = sysctl_sched_rt_period;
 	old_runtime = sysctl_sched_rt_runtime;
 
-	ret = proc_dointvec(table, write, buffer, lenp, ppos);
-
-	if (!ret && write) {
-		ret = sched_rt_global_constraints();
-		if (ret) {
-			sysctl_sched_rt_period = old_period;
-			sysctl_sched_rt_runtime = old_runtime;
-		} else {
-			def_rt_bandwidth.rt_runtime = global_rt_runtime();
-			def_rt_bandwidth.rt_period =
-				ns_to_ktime(global_rt_period());
-		}
+	ret = sched_rt_global_constraints();
+	if (ret) {
+		sysctl_sched_rt_period = old_period;
+		sysctl_sched_rt_runtime = old_runtime;
+	} else {
+		def_rt_bandwidth.rt_runtime = global_rt_runtime();
+		def_rt_bandwidth.rt_period =
+			ns_to_ktime(global_rt_period());
 	}
+
 	mutex_unlock(&mutex);
 
 	return ret;
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 23f1ac6..fad9ff6 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -347,14 +347,16 @@ static struct ctl_table kern_table[] = {
 		.data		= &sysctl_sched_rt_period,
 		.maxlen		= sizeof(unsigned int),
 		.mode		= 0644,
-		.proc_handler	= sched_rt_handler,
+		.proc_handler	= proc_dointvec,
+		.callback	= sched_rt_handler,
 	},
 	{
 		.procname	= "sched_rt_runtime_us",
 		.data		= &sysctl_sched_rt_runtime,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= sched_rt_handler,
+		.proc_handler	= proc_dointvec,
+		.callback	= sched_rt_handler,
 	},
 #ifdef CONFIG_SCHED_AUTOGROUP
 	{
-- 
1.7.8.5

Re: [PATCH 03/14] sched rt,sysctl: remove proc input checks out of sysctl handlers

[Eric Paris]

NAK

old_period = sysctl_sched_rt_period;

Doesn't make any sense in the callback, since you already updated
sysctl_sched_rt_period.

I'll leave the remainder of the series as an exercise for the reader.
But I get the feeling this isn't the only place where you are doing
things after the proc_dointvec() which must be done before.

-Eric

On Sun, Apr 29, 2012 at 2:45 AM, Sasha Levin <levinsasha928@gmail.com> wrote:
> Simplify sysctl handler by removing user input checks and using the callback
> provided by the sysctl table.
>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
>  include/linux/sched.h |    4 +---
>  kernel/sched/core.c   |   25 ++++++++++---------------
>  kernel/sysctl.c       |    6 ++++--
>  3 files changed, 15 insertions(+), 20 deletions(-)
>
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index 722da9a..9509d80 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -2152,9 +2152,7 @@ static inline unsigned int get_sysctl_timer_migration(void)
>  extern unsigned int sysctl_sched_rt_period;
>  extern int sysctl_sched_rt_runtime;
>
> -int sched_rt_handler(struct ctl_table *table, int write,
> -               void __user *buffer, size_t *lenp,
> -               loff_t *ppos);
> +int sched_rt_handler(void);
>
>  #ifdef CONFIG_SCHED_AUTOGROUP
>  extern unsigned int sysctl_sched_autogroup_enabled;
> diff --git a/kernel/sched/core.c b/kernel/sched/core.c
> index 477b998..ca4a806 100644
> --- a/kernel/sched/core.c
> +++ b/kernel/sched/core.c
> @@ -7573,9 +7573,7 @@ static int sched_rt_global_constraints(void)
>  }
>  #endif /* CONFIG_RT_GROUP_SCHED */
>
> -int sched_rt_handler(struct ctl_table *table, int write,
> -               void __user *buffer, size_t *lenp,
> -               loff_t *ppos)
> +int sched_rt_handler(void)
>  {
>        int ret;
>        int old_period, old_runtime;
> @@ -7585,19 +7583,16 @@ int sched_rt_handler(struct ctl_table *table, int write,
>        old_period = sysctl_sched_rt_period;
>        old_runtime = sysctl_sched_rt_runtime;
>
> -       ret = proc_dointvec(table, write, buffer, lenp, ppos);
> -
> -       if (!ret && write) {
> -               ret = sched_rt_global_constraints();
> -               if (ret) {
> -                       sysctl_sched_rt_period = old_period;
> -                       sysctl_sched_rt_runtime = old_runtime;
> -               } else {
> -                       def_rt_bandwidth.rt_runtime = global_rt_runtime();
> -                       def_rt_bandwidth.rt_period =
> -                               ns_to_ktime(global_rt_period());
> -               }
> +       ret = sched_rt_global_constraints();
> +       if (ret) {
> +               sysctl_sched_rt_period = old_period;
> +               sysctl_sched_rt_runtime = old_runtime;
> +       } else {
> +               def_rt_bandwidth.rt_runtime = global_rt_runtime();
> +               def_rt_bandwidth.rt_period =
> +                       ns_to_ktime(global_rt_period());
>        }
> +
>        mutex_unlock(&mutex);
>
>        return ret;
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index 23f1ac6..fad9ff6 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -347,14 +347,16 @@ static struct ctl_table kern_table[] = {
>                .data           = &sysctl_sched_rt_period,
>                .maxlen         = sizeof(unsigned int),
>                .mode           = 0644,
> -               .proc_handler   = sched_rt_handler,
> +               .proc_handler   = proc_dointvec,
> +               .callback       = sched_rt_handler,
>        },
>        {
>                .procname       = "sched_rt_runtime_us",
>                .data           = &sysctl_sched_rt_runtime,
>                .maxlen         = sizeof(int),
>                .mode           = 0644,
> -               .proc_handler   = sched_rt_handler,
> +               .proc_handler   = proc_dointvec,
> +               .callback       = sched_rt_handler,
>        },
>  #ifdef CONFIG_SCHED_AUTOGROUP
>        {
> --
> 1.7.8.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 04/14] ftrace,sysctl: remove proc input checks out of sysctl handlers

[Sasha Levin]

Simplify sysctl handler by removing user input checks and using the callback
provided by the sysctl table.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 include/linux/ftrace.h     |    8 ++------
 kernel/sysctl.c            |    6 ++++--
 kernel/trace/ftrace.c      |    8 ++------
 kernel/trace/trace_stack.c |    9 ++-------
 4 files changed, 10 insertions(+), 21 deletions(-)

diff --git a/include/linux/ftrace.h b/include/linux/ftrace.h
index 72a6cab..75a3530 100644
--- a/include/linux/ftrace.h
+++ b/include/linux/ftrace.h
@@ -25,9 +25,7 @@ struct ftrace_hash;
 
 extern int ftrace_enabled;
 extern int
-ftrace_enable_sysctl(struct ctl_table *table, int write,
-		     void __user *buffer, size_t *lenp,
-		     loff_t *ppos);
+ftrace_enable_sysctl(void);
 
 typedef void (*ftrace_func_t)(unsigned long ip, unsigned long parent_ip);
 
@@ -181,9 +179,7 @@ static inline void ftrace_start(void) { }
 #ifdef CONFIG_STACK_TRACER
 extern int stack_tracer_enabled;
 int
-stack_trace_sysctl(struct ctl_table *table, int write,
-		   void __user *buffer, size_t *lenp,
-		   loff_t *ppos);
+stack_trace_sysctl(void);
 #endif
 
 struct ftrace_func_command {
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index fad9ff6..40be238 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -519,7 +519,8 @@ static struct ctl_table kern_table[] = {
 		.data		= &ftrace_enabled,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= ftrace_enable_sysctl,
+		.proc_handler	= proc_dointvec,
+		.callback	= ftrace_enable_sysctl,
 	},
 #endif
 #ifdef CONFIG_STACK_TRACER
@@ -528,7 +529,8 @@ static struct ctl_table kern_table[] = {
 		.data		= &stack_tracer_enabled,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= stack_trace_sysctl,
+		.callback	= stack_trace_sysctl,
+		.proc_handler	= proc_dointvec,
 	},
 #endif
 #ifdef CONFIG_TRACING
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
index 0fa92f6..70a5ec4 100644
--- a/kernel/trace/ftrace.c
+++ b/kernel/trace/ftrace.c
@@ -4340,9 +4340,7 @@ int unregister_ftrace_function(struct ftrace_ops *ops)
 EXPORT_SYMBOL_GPL(unregister_ftrace_function);
 
 int
-ftrace_enable_sysctl(struct ctl_table *table, int write,
-		     void __user *buffer, size_t *lenp,
-		     loff_t *ppos)
+ftrace_enable_sysctl(void)
 {
 	int ret = -ENODEV;
 
@@ -4351,9 +4349,7 @@ ftrace_enable_sysctl(struct ctl_table *table, int write,
 	if (unlikely(ftrace_disabled))
 		goto out;
 
-	ret = proc_dointvec(table, write, buffer, lenp, ppos);
-
-	if (ret || !write || (last_ftrace_enabled == !!ftrace_enabled))
+	if (last_ftrace_enabled == !!ftrace_enabled)
 		goto out;
 
 	last_ftrace_enabled = !!ftrace_enabled;
diff --git a/kernel/trace/trace_stack.c b/kernel/trace/trace_stack.c
index d4545f4..3b05ec2 100644
--- a/kernel/trace/trace_stack.c
+++ b/kernel/trace/trace_stack.c
@@ -329,18 +329,13 @@ static const struct file_operations stack_trace_filter_fops = {
 };
 
 int
-stack_trace_sysctl(struct ctl_table *table, int write,
-		   void __user *buffer, size_t *lenp,
-		   loff_t *ppos)
+stack_trace_sysctl(void)
 {
 	int ret;
 
 	mutex_lock(&stack_sysctl_mutex);
 
-	ret = proc_dointvec(table, write, buffer, lenp, ppos);
-
-	if (ret || !write ||
-	    (last_stack_tracer_enabled == !!stack_tracer_enabled))
+	if (last_stack_tracer_enabled == !!stack_tracer_enabled)
 		goto out;
 
 	last_stack_tracer_enabled = !!stack_tracer_enabled;
-- 
1.7.8.5

[PATCH 05/14] sysrq,sysctl: remove proc input checks out of sysctl handlers

[Sasha Levin]

Simplify sysctl handler by removing user input checks and using the callback
provided by the sysctl table.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 kernel/sysctl.c |   16 ++++------------
 1 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 40be238..bde6087 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -178,18 +178,9 @@ static int proc_dointvec_minmax_sysadmin(struct ctl_table *table, int write,
 /* Note: sysrq code uses it's own private copy */
 static int __sysrq_enabled = SYSRQ_DEFAULT_ENABLE;
 
-static int sysrq_sysctl_handler(ctl_table *table, int write,
-				void __user *buffer, size_t *lenp,
-				loff_t *ppos)
+static int sysrq_sysctl_handler(void)
 {
-	int error;
-
-	error = proc_dointvec(table, write, buffer, lenp, ppos);
-	if (error)
-		return error;
-
-	if (write)
-		sysrq_toggle_support(__sysrq_enabled);
+	sysrq_toggle_support(__sysrq_enabled);
 
 	return 0;
 }
@@ -594,7 +585,8 @@ static struct ctl_table kern_table[] = {
 		.data		= &__sysrq_enabled,
 		.maxlen		= sizeof (int),
 		.mode		= 0644,
-		.proc_handler	= sysrq_sysctl_handler,
+		.proc_handler	= proc_dointvec,
+		.callback	= sysrq_sysctl_handler,
 	},
 #endif
 #ifdef CONFIG_PROC_SYSCTL
-- 
1.7.8.5

[PATCH 06/14] watchdog,sysctl: remove unused external

[Sasha Levin]

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 include/linux/sched.h |    3 ---
 1 files changed, 0 insertions(+), 3 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 9509d80..22e3768 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -312,9 +312,6 @@ extern void sched_show_task(struct task_struct *p);
 extern void touch_softlockup_watchdog(void);
 extern void touch_softlockup_watchdog_sync(void);
 extern void touch_all_softlockup_watchdogs(void);
-extern int proc_dowatchdog_thresh(struct ctl_table *table, int write,
-				  void __user *buffer,
-				  size_t *lenp, loff_t *ppos);
 extern unsigned int  softlockup_panic;
 void lockup_detector_init(void);
 #else
-- 
1.7.8.5

[PATCH 07/14] watchdog,sysctl: remove proc input checks out of sysctl handlers

[Sasha Levin]

Simplify sysctl handler by removing user input checks and using the callback
provided by the sysctl table.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 include/linux/nmi.h |    3 +--
 kernel/sysctl.c     |    9 ++++++---
 kernel/watchdog.c   |   12 ++----------
 3 files changed, 9 insertions(+), 15 deletions(-)

diff --git a/include/linux/nmi.h b/include/linux/nmi.h
index db50840..7a8030d 100644
--- a/include/linux/nmi.h
+++ b/include/linux/nmi.h
@@ -49,8 +49,7 @@ u64 hw_nmi_get_sample_period(int watchdog_thresh);
 extern int watchdog_enabled;
 extern int watchdog_thresh;
 struct ctl_table;
-extern int proc_dowatchdog(struct ctl_table *, int ,
-			   void __user *, size_t *, loff_t *);
+extern int proc_dowatchdog(void);
 #endif
 
 #endif
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index bde6087..2fac00a 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -737,7 +737,8 @@ static struct ctl_table kern_table[] = {
 		.data           = &watchdog_enabled,
 		.maxlen         = sizeof (int),
 		.mode           = 0644,
-		.proc_handler   = proc_dowatchdog,
+		.proc_handler	= proc_dointvec_minmax,
+		.callback	= proc_dowatchdog,
 		.extra1		= &zero,
 		.extra2		= &one,
 	},
@@ -746,7 +747,8 @@ static struct ctl_table kern_table[] = {
 		.data		= &watchdog_thresh,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= proc_dowatchdog,
+		.proc_handler	= proc_dointvec_minmax,
+		.callback	= proc_dowatchdog,
 		.extra1		= &neg_one,
 		.extra2		= &sixty,
 	},
@@ -764,7 +766,8 @@ static struct ctl_table kern_table[] = {
 		.data           = &watchdog_enabled,
 		.maxlen         = sizeof (int),
 		.mode           = 0644,
-		.proc_handler   = proc_dowatchdog,
+		.proc_handler	= proc_dointvec_minmax,
+		.callback	= proc_dowatchdog,
 		.extra1		= &zero,
 		.extra2		= &one,
 	},
diff --git a/kernel/watchdog.c b/kernel/watchdog.c
index e5e1d85..56bef16 100644
--- a/kernel/watchdog.c
+++ b/kernel/watchdog.c
@@ -535,22 +535,14 @@ static void watchdog_disable_all_cpus(void)
  * proc handler for /proc/sys/kernel/nmi_watchdog,watchdog_thresh
  */
 
-int proc_dowatchdog(struct ctl_table *table, int write,
-		    void __user *buffer, size_t *lenp, loff_t *ppos)
+int proc_dowatchdog(void)
 {
-	int ret;
-
-	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
-	if (ret || !write)
-		goto out;
-
 	if (watchdog_enabled && watchdog_thresh)
 		watchdog_enable_all_cpus();
 	else
 		watchdog_disable_all_cpus();
 
-out:
-	return ret;
+	return 0;
 }
 #endif /* CONFIG_SYSCTL */
 
-- 
1.7.8.5

[PATCH 08/14] hung task,sysctl: remove proc input checks out of sysctl handlers

[Sasha Levin]

Simplify sysctl handler by removing user input checks and using the callback
provided by the sysctl table.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 include/linux/sched.h |    4 +---
 kernel/hung_task.c    |   14 ++------------
 kernel/sysctl.c       |    3 ++-
 3 files changed, 5 insertions(+), 16 deletions(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index 22e3768..f148b98 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -334,9 +334,7 @@ extern unsigned int  sysctl_hung_task_panic;
 extern unsigned long sysctl_hung_task_check_count;
 extern unsigned long sysctl_hung_task_timeout_secs;
 extern unsigned long sysctl_hung_task_warnings;
-extern int proc_dohung_task_timeout_secs(struct ctl_table *table, int write,
-					 void __user *buffer,
-					 size_t *lenp, loff_t *ppos);
+extern int proc_dohung_task_timeout_secs(void);
 #else
 /* Avoid need for ifdefs elsewhere in the code */
 enum { sysctl_hung_task_timeout_secs = 0 };
diff --git a/kernel/hung_task.c b/kernel/hung_task.c
index 6df6149..5c67710 100644
--- a/kernel/hung_task.c
+++ b/kernel/hung_task.c
@@ -181,21 +181,11 @@ static unsigned long timeout_jiffies(unsigned long timeout)
 /*
  * Process updating of timeout sysctl
  */
-int proc_dohung_task_timeout_secs(struct ctl_table *table, int write,
-				  void __user *buffer,
-				  size_t *lenp, loff_t *ppos)
+int proc_dohung_task_timeout_secs(void)
 {
-	int ret;
-
-	ret = proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
-
-	if (ret || !write)
-		goto out;
-
 	wake_up_process(watchdog_task);
 
- out:
-	return ret;
+	return 0;
 }
 
 /*
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 2fac00a..16252c9 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -899,7 +899,8 @@ static struct ctl_table kern_table[] = {
 		.data		= &sysctl_hung_task_timeout_secs,
 		.maxlen		= sizeof(unsigned long),
 		.mode		= 0644,
-		.proc_handler	= proc_dohung_task_timeout_secs,
+		.proc_handler	= proc_doulongvec_minmax,
+		.callback	= proc_dohung_task_timeout_secs,
 	},
 	{
 		.procname	= "hung_task_warnings",
-- 
1.7.8.5

[PATCH 09/14] perf,sysctl: remove proc input checks out of sysctl handlers

[Sasha Levin]

Simplify sysctl handler by removing user input checks and using the callback
provided by the sysctl table.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 include/linux/perf_event.h |    4 +---
 kernel/events/core.c       |    9 +--------
 kernel/sysctl.c            |    3 ++-
 3 files changed, 4 insertions(+), 12 deletions(-)

diff --git a/include/linux/perf_event.h b/include/linux/perf_event.h
index ddbb6a9..24c3a54 100644
--- a/include/linux/perf_event.h
+++ b/include/linux/perf_event.h
@@ -1244,9 +1244,7 @@ extern int sysctl_perf_event_paranoid;
 extern int sysctl_perf_event_mlock;
 extern int sysctl_perf_event_sample_rate;
 
-extern int perf_proc_update_handler(struct ctl_table *table, int write,
-		void __user *buffer, size_t *lenp,
-		loff_t *ppos);
+extern int perf_proc_update_handler(void);
 
 static inline bool perf_paranoid_tracepoint_raw(void)
 {
diff --git a/kernel/events/core.c b/kernel/events/core.c
index 32cfc76..94d126f 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -167,15 +167,8 @@ int sysctl_perf_event_sample_rate __read_mostly = DEFAULT_MAX_SAMPLE_RATE;
 static int max_samples_per_tick __read_mostly =
 	DIV_ROUND_UP(DEFAULT_MAX_SAMPLE_RATE, HZ);
 
-int perf_proc_update_handler(struct ctl_table *table, int write,
-		void __user *buffer, size_t *lenp,
-		loff_t *ppos)
+int perf_proc_update_handler(void)
 {
-	int ret = proc_dointvec(table, write, buffer, lenp, ppos);
-
-	if (ret || !write)
-		return ret;
-
 	max_samples_per_tick = DIV_ROUND_UP(sysctl_perf_event_sample_rate, HZ);
 
 	return 0;
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 16252c9..eef7508 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -977,7 +977,8 @@ static struct ctl_table kern_table[] = {
 		.data		= &sysctl_perf_event_sample_rate,
 		.maxlen		= sizeof(sysctl_perf_event_sample_rate),
 		.mode		= 0644,
-		.proc_handler	= perf_proc_update_handler,
+		.proc_handler	= proc_dointvec,
+		.callback	= perf_proc_update_handler,
 	},
 #endif
 #ifdef CONFIG_KMEMCHECK
-- 
1.7.8.5

[PATCH 10/14] mm,sysctl: remove proc input checks out of sysctl handlers

[Sasha Levin]

Simplify sysctl handler by removing user input checks and using the callback
provided by the sysctl table.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 include/linux/mmzone.h    |   15 ++++---------
 include/linux/writeback.h |   19 ++++-------------
 kernel/sysctl.c           |   30 ++++++++++++++++++---------
 mm/page-writeback.c       |   48 ++++++++++++--------------------------------
 mm/page_alloc.c           |   37 ++++++----------------------------
 5 files changed, 50 insertions(+), 99 deletions(-)

diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 5c4880b..52fc184 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -824,17 +824,12 @@ static inline int is_dma(struct zone *zone)
 
 /* These two functions are used to setup the per zone pages min values */
 struct ctl_table;
-int min_free_kbytes_sysctl_handler(struct ctl_table *, int,
-					void __user *, size_t *, loff_t *);
+int min_free_kbytes_sysctl_handler(void);
 extern int sysctl_lowmem_reserve_ratio[MAX_NR_ZONES-1];
-int lowmem_reserve_ratio_sysctl_handler(struct ctl_table *, int,
-					void __user *, size_t *, loff_t *);
-int percpu_pagelist_fraction_sysctl_handler(struct ctl_table *, int,
-					void __user *, size_t *, loff_t *);
-int sysctl_min_unmapped_ratio_sysctl_handler(struct ctl_table *, int,
-			void __user *, size_t *, loff_t *);
-int sysctl_min_slab_ratio_sysctl_handler(struct ctl_table *, int,
-			void __user *, size_t *, loff_t *);
+int lowmem_reserve_ratio_sysctl_handler(void);
+int percpu_pagelist_fraction_sysctl_handler(void);
+int sysctl_min_unmapped_ratio_sysctl_handler(void);
+int sysctl_min_slab_ratio_sysctl_handler(void);
 
 extern int numa_zonelist_order_handler(struct ctl_table *, int,
 			void __user *, size_t *, loff_t *);
diff --git a/include/linux/writeback.h b/include/linux/writeback.h
index 3309736..9081cf1 100644
--- a/include/linux/writeback.h
+++ b/include/linux/writeback.h
@@ -137,22 +137,13 @@ extern int vm_highmem_is_dirtyable;
 extern int block_dump;
 extern int laptop_mode;
 
-extern int dirty_background_ratio_handler(struct ctl_table *table, int write,
-		void __user *buffer, size_t *lenp,
-		loff_t *ppos);
-extern int dirty_background_bytes_handler(struct ctl_table *table, int write,
-		void __user *buffer, size_t *lenp,
-		loff_t *ppos);
-extern int dirty_ratio_handler(struct ctl_table *table, int write,
-		void __user *buffer, size_t *lenp,
-		loff_t *ppos);
-extern int dirty_bytes_handler(struct ctl_table *table, int write,
-		void __user *buffer, size_t *lenp,
-		loff_t *ppos);
+extern int dirty_background_ratio_handler(void);
+extern int dirty_background_bytes_handler(void);
+extern int dirty_ratio_handler(void);
+extern int dirty_bytes_handler(void);
 
 struct ctl_table;
-int dirty_writeback_centisecs_handler(struct ctl_table *, int,
-				      void __user *, size_t *, loff_t *);
+int dirty_writeback_centisecs_handler(void);
 
 void global_dirty_limits(unsigned long *pbackground, unsigned long *pdirty);
 unsigned long bdi_dirty_limit(struct backing_dev_info *bdi,
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index eef7508..3c403fd 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1055,7 +1055,8 @@ static struct ctl_table vm_table[] = {
 		.data		= &dirty_background_ratio,
 		.maxlen		= sizeof(dirty_background_ratio),
 		.mode		= 0644,
-		.proc_handler	= dirty_background_ratio_handler,
+		.proc_handler	= proc_dointvec_minmax,
+		.callback	= dirty_background_ratio_handler,
 		.extra1		= &zero,
 		.extra2		= &one_hundred,
 	},
@@ -1064,7 +1065,8 @@ static struct ctl_table vm_table[] = {
 		.data		= &dirty_background_bytes,
 		.maxlen		= sizeof(dirty_background_bytes),
 		.mode		= 0644,
-		.proc_handler	= dirty_background_bytes_handler,
+		.proc_handler	= proc_doulongvec_minmax,
+		.callback	= dirty_background_bytes_handler,
 		.extra1		= &one_ul,
 	},
 	{
@@ -1072,7 +1074,8 @@ static struct ctl_table vm_table[] = {
 		.data		= &vm_dirty_ratio,
 		.maxlen		= sizeof(vm_dirty_ratio),
 		.mode		= 0644,
-		.proc_handler	= dirty_ratio_handler,
+		.proc_handler	= proc_dointvec_minmax,
+		.callback	= dirty_ratio_handler,
 		.extra1		= &zero,
 		.extra2		= &one_hundred,
 	},
@@ -1081,7 +1084,8 @@ static struct ctl_table vm_table[] = {
 		.data		= &vm_dirty_bytes,
 		.maxlen		= sizeof(vm_dirty_bytes),
 		.mode		= 0644,
-		.proc_handler	= dirty_bytes_handler,
+		.proc_handler	= proc_doulongvec_minmax,
+		.callback	= dirty_bytes_handler,
 		.extra1		= &dirty_bytes_min,
 	},
 	{
@@ -1089,7 +1093,8 @@ static struct ctl_table vm_table[] = {
 		.data		= &dirty_writeback_interval,
 		.maxlen		= sizeof(dirty_writeback_interval),
 		.mode		= 0644,
-		.proc_handler	= dirty_writeback_centisecs_handler,
+		.proc_handler	= proc_dointvec,
+		.callback	= dirty_writeback_centisecs_handler,
 	},
 	{
 		.procname	= "dirty_expire_centisecs",
@@ -1165,7 +1170,8 @@ static struct ctl_table vm_table[] = {
 		.data		= &sysctl_lowmem_reserve_ratio,
 		.maxlen		= sizeof(sysctl_lowmem_reserve_ratio),
 		.mode		= 0644,
-		.proc_handler	= lowmem_reserve_ratio_sysctl_handler,
+		.proc_handler	= proc_dointvec_minmax,
+		.callback	= lowmem_reserve_ratio_sysctl_handler,
 	},
 	{
 		.procname	= "drop_caches",
@@ -1200,7 +1206,8 @@ static struct ctl_table vm_table[] = {
 		.data		= &min_free_kbytes,
 		.maxlen		= sizeof(min_free_kbytes),
 		.mode		= 0644,
-		.proc_handler	= min_free_kbytes_sysctl_handler,
+		.proc_handler	= proc_dointvec,
+		.callback	= min_free_kbytes_sysctl_handler,
 		.extra1		= &zero,
 	},
 	{
@@ -1208,7 +1215,8 @@ static struct ctl_table vm_table[] = {
 		.data		= &percpu_pagelist_fraction,
 		.maxlen		= sizeof(percpu_pagelist_fraction),
 		.mode		= 0644,
-		.proc_handler	= percpu_pagelist_fraction_sysctl_handler,
+		.proc_handler	= proc_dointvec_minmax,
+		.callback	= percpu_pagelist_fraction_sysctl_handler,
 		.extra1		= &min_percpu_pagelist_fract,
 	},
 #ifdef CONFIG_MMU
@@ -1277,7 +1285,8 @@ static struct ctl_table vm_table[] = {
 		.data		= &sysctl_min_unmapped_ratio,
 		.maxlen		= sizeof(sysctl_min_unmapped_ratio),
 		.mode		= 0644,
-		.proc_handler	= sysctl_min_unmapped_ratio_sysctl_handler,
+		.proc_handler	= proc_dointvec_minmax,
+		.callback	= sysctl_min_unmapped_ratio_sysctl_handler,
 		.extra1		= &zero,
 		.extra2		= &one_hundred,
 	},
@@ -1286,7 +1295,8 @@ static struct ctl_table vm_table[] = {
 		.data		= &sysctl_min_slab_ratio,
 		.maxlen		= sizeof(sysctl_min_slab_ratio),
 		.mode		= 0644,
-		.proc_handler	= sysctl_min_slab_ratio_sysctl_handler,
+		.proc_handler	= proc_dointvec_minmax,
+		.callback	= sysctl_min_slab_ratio_sysctl_handler,
 		.extra1		= &zero,
 		.extra2		= &one_hundred,
 	},
diff --git a/mm/page-writeback.c b/mm/page-writeback.c
index 9dec97f..3898114 100644
--- a/mm/page-writeback.c
+++ b/mm/page-writeback.c
@@ -350,58 +350,38 @@ static void update_completion_period(void)
 	writeback_set_ratelimit();
 }
 
-int dirty_background_ratio_handler(struct ctl_table *table, int write,
-		void __user *buffer, size_t *lenp,
-		loff_t *ppos)
+int dirty_background_ratio_handler(void)
 {
-	int ret;
-
-	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
-	if (ret == 0 && write)
-		dirty_background_bytes = 0;
-	return ret;
+	dirty_background_bytes = 0;
+	return 0;
 }
 
-int dirty_background_bytes_handler(struct ctl_table *table, int write,
-		void __user *buffer, size_t *lenp,
-		loff_t *ppos)
+int dirty_background_bytes_handler(void)
 {
-	int ret;
-
-	ret = proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
-	if (ret == 0 && write)
-		dirty_background_ratio = 0;
-	return ret;
+	dirty_background_ratio = 0;
+	return 0;
 }
 
-int dirty_ratio_handler(struct ctl_table *table, int write,
-		void __user *buffer, size_t *lenp,
-		loff_t *ppos)
+int dirty_ratio_handler(void)
 {
 	int old_ratio = vm_dirty_ratio;
-	int ret;
 
-	ret = proc_dointvec_minmax(table, write, buffer, lenp, ppos);
-	if (ret == 0 && write && vm_dirty_ratio != old_ratio) {
+	if (vm_dirty_ratio != old_ratio) {
 		update_completion_period();
 		vm_dirty_bytes = 0;
 	}
-	return ret;
+	return 0;
 }
 
-int dirty_bytes_handler(struct ctl_table *table, int write,
-		void __user *buffer, size_t *lenp,
-		loff_t *ppos)
+int dirty_bytes_handler(void)
 {
 	unsigned long old_bytes = vm_dirty_bytes;
-	int ret;
 
-	ret = proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
-	if (ret == 0 && write && vm_dirty_bytes != old_bytes) {
+	if (vm_dirty_bytes != old_bytes) {
 		update_completion_period();
 		vm_dirty_ratio = 0;
 	}
-	return ret;
+	return 0;
 }
 
 /*
@@ -1500,10 +1480,8 @@ void throttle_vm_writeout(gfp_t gfp_mask)
 /*
  * sysctl handler for /proc/sys/vm/dirty_writeback_centisecs
  */
-int dirty_writeback_centisecs_handler(ctl_table *table, int write,
-	void __user *buffer, size_t *length, loff_t *ppos)
+int dirty_writeback_centisecs_handler(void)
 {
-	proc_dointvec(table, write, buffer, length, ppos);
 	bdi_arm_supers_timer();
 	return 0;
 }
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 1b951de..1638fc1 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5193,29 +5193,19 @@ int __meminit init_per_zone_wmark_min(void)
 module_init(init_per_zone_wmark_min)
 
 /*
- * min_free_kbytes_sysctl_handler - just a wrapper around proc_dointvec() so 
- *	that we can call two helper functions whenever min_free_kbytes
+ * min_free_kbytes_sysctl_handler - called whenever min_free_kbytes
  *	changes.
  */
-int min_free_kbytes_sysctl_handler(ctl_table *table, int write, 
-	void __user *buffer, size_t *length, loff_t *ppos)
+int min_free_kbytes_sysctl_handler(void)
 {
-	proc_dointvec(table, write, buffer, length, ppos);
-	if (write)
-		setup_per_zone_wmarks();
+	setup_per_zone_wmarks();
 	return 0;
 }
 
 #ifdef CONFIG_NUMA
-int sysctl_min_unmapped_ratio_sysctl_handler(ctl_table *table, int write,
-	void __user *buffer, size_t *length, loff_t *ppos)
+int sysctl_min_unmapped_ratio_sysctl_handler(void)
 {
 	struct zone *zone;
-	int rc;
-
-	rc = proc_dointvec_minmax(table, write, buffer, length, ppos);
-	if (rc)
-		return rc;
 
 	for_each_zone(zone)
 		zone->min_unmapped_pages = (zone->present_pages *
@@ -5223,15 +5213,9 @@ int sysctl_min_unmapped_ratio_sysctl_handler(ctl_table *table, int write,
 	return 0;
 }
 
-int sysctl_min_slab_ratio_sysctl_handler(ctl_table *table, int write,
-	void __user *buffer, size_t *length, loff_t *ppos)
+int sysctl_min_slab_ratio_sysctl_handler(void)
 {
 	struct zone *zone;
-	int rc;
-
-	rc = proc_dointvec_minmax(table, write, buffer, length, ppos);
-	if (rc)
-		return rc;
 
 	for_each_zone(zone)
 		zone->min_slab_pages = (zone->present_pages *
@@ -5249,10 +5233,8 @@ int sysctl_min_slab_ratio_sysctl_handler(ctl_table *table, int write,
  * minimum watermarks. The lowmem reserve ratio can only make sense
  * if in function of the boot time zone sizes.
  */
-int lowmem_reserve_ratio_sysctl_handler(ctl_table *table, int write,
-	void __user *buffer, size_t *length, loff_t *ppos)
+int lowmem_reserve_ratio_sysctl_handler(void)
 {
-	proc_dointvec_minmax(table, write, buffer, length, ppos);
 	setup_per_zone_lowmem_reserve();
 	return 0;
 }
@@ -5263,16 +5245,11 @@ int lowmem_reserve_ratio_sysctl_handler(ctl_table *table, int write,
  * can have before it gets flushed back to buddy allocator.
  */
 
-int percpu_pagelist_fraction_sysctl_handler(ctl_table *table, int write,
-	void __user *buffer, size_t *length, loff_t *ppos)
+int percpu_pagelist_fraction_sysctl_handler(void)
 {
 	struct zone *zone;
 	unsigned int cpu;
-	int ret;
 
-	ret = proc_dointvec_minmax(table, write, buffer, length, ppos);
-	if (!write || (ret == -EINVAL))
-		return ret;
 	for_each_populated_zone(zone) {
 		for_each_possible_cpu(cpu) {
 			unsigned long  high;
-- 
1.7.8.5

[PATCH 11/14] hugetlb,sysctl: remove proc input checks out of sysctl handlers

[Sasha Levin]

Simplify sysctl handler by removing user input checks and using the callback
provided by the sysctl table.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 include/linux/hugetlb.h |    2 +-
 kernel/sysctl.c         |    3 ++-
 mm/hugetlb.c            |    5 +----
 3 files changed, 4 insertions(+), 6 deletions(-)

diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
index a2e45ab..8657de4 100644
--- a/include/linux/hugetlb.h
+++ b/include/linux/hugetlb.h
@@ -30,7 +30,7 @@ int PageHuge(struct page *page);
 void reset_vma_resv_huge_pages(struct vm_area_struct *vma);
 int hugetlb_sysctl_handler(struct ctl_table *, int, void __user *, size_t *, loff_t *);
 int hugetlb_overcommit_handler(struct ctl_table *, int, void __user *, size_t *, loff_t *);
-int hugetlb_treat_movable_handler(struct ctl_table *, int, void __user *, size_t *, loff_t *);
+int hugetlb_treat_movable_handler(void);
 
 #ifdef CONFIG_NUMA
 int hugetlb_mempolicy_sysctl_handler(struct ctl_table *, int,
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 3c403fd..dacece7 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1153,7 +1153,8 @@ static struct ctl_table vm_table[] = {
 		.data		= &hugepages_treat_as_movable,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= hugetlb_treat_movable_handler,
+		.proc_handler	= proc_dointvec,
+		.callback	= hugetlb_treat_movable_handler,
 	},
 	{
 		.procname	= "nr_overcommit_hugepages",
diff --git a/mm/hugetlb.c b/mm/hugetlb.c
index bf84ae3..2d896ee 100644
--- a/mm/hugetlb.c
+++ b/mm/hugetlb.c
@@ -2089,11 +2089,8 @@ int hugetlb_mempolicy_sysctl_handler(struct ctl_table *table, int write,
 }
 #endif /* CONFIG_NUMA */
 
-int hugetlb_treat_movable_handler(struct ctl_table *table, int write,
-			void __user *buffer,
-			size_t *length, loff_t *ppos)
+int hugetlb_treat_movable_handler(void)
 {
-	proc_dointvec(table, write, buffer, length, ppos);
 	if (hugepages_treat_as_movable)
 		htlb_alloc_mask = GFP_HIGHUSER_MOVABLE;
 	else
-- 
1.7.8.5

[PATCH 12/14] mm compaction,sysctl: remove proc input checks out of sysctl handlers

[Sasha Levin]

Simplify sysctl handler by removing user input checks and using the callback
provided by the sysctl table.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 include/linux/compaction.h |    2 --
 kernel/sysctl.c            |    2 +-
 mm/compaction.c            |    8 --------
 3 files changed, 1 insertions(+), 11 deletions(-)

diff --git a/include/linux/compaction.h b/include/linux/compaction.h
index 51a90b7..35788e4 100644
--- a/include/linux/compaction.h
+++ b/include/linux/compaction.h
@@ -16,8 +16,6 @@ extern int sysctl_compact_memory;
 extern int sysctl_compaction_handler(struct ctl_table *table, int write,
 			void __user *buffer, size_t *length, loff_t *ppos);
 extern int sysctl_extfrag_threshold;
-extern int sysctl_extfrag_handler(struct ctl_table *table, int write,
-			void __user *buffer, size_t *length, loff_t *ppos);
 
 extern int fragmentation_index(struct zone *zone, unsigned int order);
 extern unsigned long try_to_compact_pages(struct zonelist *zonelist,
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index dacece7..f9ce79b 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1196,7 +1196,7 @@ static struct ctl_table vm_table[] = {
 		.data		= &sysctl_extfrag_threshold,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= sysctl_extfrag_handler,
+		.proc_handler	= proc_dointvec_minmax,
 		.extra1		= &min_extfrag_threshold,
 		.extra2		= &max_extfrag_threshold,
 	},
diff --git a/mm/compaction.c b/mm/compaction.c
index 58f7a93..74c44f2 100644
--- a/mm/compaction.c
+++ b/mm/compaction.c
@@ -863,14 +863,6 @@ int sysctl_compaction_handler(struct ctl_table *table, int write,
 	return 0;
 }
 
-int sysctl_extfrag_handler(struct ctl_table *table, int write,
-			void __user *buffer, size_t *length, loff_t *ppos)
-{
-	proc_dointvec_minmax(table, write, buffer, length, ppos);
-
-	return 0;
-}
-
 #if defined(CONFIG_SYSFS) && defined(CONFIG_NUMA)
 ssize_t sysfs_compact_node(struct device *dev,
 			struct device_attribute *attr,
-- 
1.7.8.5

[PATCH 13/14] security,sysctl: remove proc input checks out of sysctl handlers

[Sasha Levin]

Simplify sysctl handler by removing user input checks and using the callback
provided by the sysctl table.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 include/linux/security.h |    3 +--
 kernel/sysctl.c          |    3 ++-
 security/min_addr.c      |   11 +++--------
 3 files changed, 6 insertions(+), 11 deletions(-)

diff --git a/include/linux/security.h b/include/linux/security.h
index ab0e091..3d3445c 100644
--- a/include/linux/security.h
+++ b/include/linux/security.h
@@ -147,8 +147,7 @@ struct request_sock;
 #define LSM_UNSAFE_NO_NEW_PRIVS	8
 
 #ifdef CONFIG_MMU
-extern int mmap_min_addr_handler(struct ctl_table *table, int write,
-				 void __user *buffer, size_t *lenp, loff_t *ppos);
+extern int mmap_min_addr_handler(void);
 #endif
 
 /* security_inode_init_security callback function to write xattrs */
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index f9ce79b..2104452 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1317,7 +1317,8 @@ static struct ctl_table vm_table[] = {
 		.data		= &dac_mmap_min_addr,
 		.maxlen		= sizeof(unsigned long),
 		.mode		= 0644,
-		.proc_handler	= mmap_min_addr_handler,
+		.proc_handler	= proc_doulongvec_minmax,
+		.callback	= mmap_min_addr_handler,
 	},
 #endif
 #ifdef CONFIG_NUMA
diff --git a/security/min_addr.c b/security/min_addr.c
index f728728..3e5a41c 100644
--- a/security/min_addr.c
+++ b/security/min_addr.c
@@ -28,19 +28,14 @@ static void update_mmap_min_addr(void)
  * sysctl handler which just sets dac_mmap_min_addr = the new value and then
  * calls update_mmap_min_addr() so non MAP_FIXED hints get rounded properly
  */
-int mmap_min_addr_handler(struct ctl_table *table, int write,
-			  void __user *buffer, size_t *lenp, loff_t *ppos)
+int mmap_min_addr_handler(void)
 {
-	int ret;
-
-	if (write && !capable(CAP_SYS_RAWIO))
+	if (!capable(CAP_SYS_RAWIO))
 		return -EPERM;
 
-	ret = proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
-
 	update_mmap_min_addr();
 
-	return ret;
+	return 0;
 }
 
 static int __init init_mmap_min_addr(void)
-- 
1.7.8.5

Re: [PATCH 13/14] security,sysctl: remove proc input checks out of sysctl handlers

[Eric Paris]

NAK - You moved the check to see if someone has permission to make a
change AFTER the change was made.  The original semantics were
correct.  You must do the capable check, then update the value, then
do the other calculations with the new value.  You can't do the
permission check after you already made the changes.

-Eric

On Sun, Apr 29, 2012 at 2:45 AM, Sasha Levin <levinsasha928@gmail.com> wrote:
> Simplify sysctl handler by removing user input checks and using the callback
> provided by the sysctl table.
>
> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
>  include/linux/security.h |    3 +--
>  kernel/sysctl.c          |    3 ++-
>  security/min_addr.c      |   11 +++--------
>  3 files changed, 6 insertions(+), 11 deletions(-)
>
> diff --git a/include/linux/security.h b/include/linux/security.h
> index ab0e091..3d3445c 100644
> --- a/include/linux/security.h
> +++ b/include/linux/security.h
> @@ -147,8 +147,7 @@ struct request_sock;
>  #define LSM_UNSAFE_NO_NEW_PRIVS        8
>
>  #ifdef CONFIG_MMU
> -extern int mmap_min_addr_handler(struct ctl_table *table, int write,
> -                                void __user *buffer, size_t *lenp, loff_t *ppos);
> +extern int mmap_min_addr_handler(void);
>  #endif
>
>  /* security_inode_init_security callback function to write xattrs */
> diff --git a/kernel/sysctl.c b/kernel/sysctl.c
> index f9ce79b..2104452 100644
> --- a/kernel/sysctl.c
> +++ b/kernel/sysctl.c
> @@ -1317,7 +1317,8 @@ static struct ctl_table vm_table[] = {
>                .data           = &dac_mmap_min_addr,
>                .maxlen         = sizeof(unsigned long),
>                .mode           = 0644,
> -               .proc_handler   = mmap_min_addr_handler,
> +               .proc_handler   = proc_doulongvec_minmax,
> +               .callback       = mmap_min_addr_handler,
>        },
>  #endif
>  #ifdef CONFIG_NUMA
> diff --git a/security/min_addr.c b/security/min_addr.c
> index f728728..3e5a41c 100644
> --- a/security/min_addr.c
> +++ b/security/min_addr.c
> @@ -28,19 +28,14 @@ static void update_mmap_min_addr(void)
>  * sysctl handler which just sets dac_mmap_min_addr = the new value and then
>  * calls update_mmap_min_addr() so non MAP_FIXED hints get rounded properly
>  */
> -int mmap_min_addr_handler(struct ctl_table *table, int write,
> -                         void __user *buffer, size_t *lenp, loff_t *ppos)
> +int mmap_min_addr_handler(void)
>  {
> -       int ret;
> -
> -       if (write && !capable(CAP_SYS_RAWIO))
> +       if (!capable(CAP_SYS_RAWIO))
>                return -EPERM;
>
> -       ret = proc_doulongvec_minmax(table, write, buffer, lenp, ppos);
> -
>        update_mmap_min_addr();
>
> -       return ret;
> +       return 0;
>  }
>
>  static int __init init_mmap_min_addr(void)
> --
> 1.7.8.5
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at  http://www.tux.org/lkml/

[PATCH 14/14] fs,sysctl: remove proc input checks out of sysctl handlers

[Sasha Levin]

Simplify sysctl handler by removing user input checks and using the callback
provided by the sysctl table.

Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
---
 fs/pipe.c                 |   11 ++---------
 include/linux/pipe_fs_i.h |    2 +-
 kernel/sysctl.c           |    3 ++-
 3 files changed, 5 insertions(+), 11 deletions(-)

diff --git a/fs/pipe.c b/fs/pipe.c
index 25feaa3..7bb7395 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -1186,17 +1186,10 @@ static inline unsigned int round_pipe_size(unsigned int size)
  * This should work even if CONFIG_PROC_FS isn't set, as proc_dointvec_minmax
  * will return an error.
  */
-int pipe_proc_fn(struct ctl_table *table, int write, void __user *buf,
-		 size_t *lenp, loff_t *ppos)
+int pipe_proc_fn(void)
 {
-	int ret;
-
-	ret = proc_dointvec_minmax(table, write, buf, lenp, ppos);
-	if (ret < 0 || !write)
-		return ret;
-
 	pipe_max_size = round_pipe_size(pipe_max_size);
-	return ret;
+	return 0;
 }
 
 /*
diff --git a/include/linux/pipe_fs_i.h b/include/linux/pipe_fs_i.h
index 6d626ff..08e02d8 100644
--- a/include/linux/pipe_fs_i.h
+++ b/include/linux/pipe_fs_i.h
@@ -139,7 +139,7 @@ void pipe_unlock(struct pipe_inode_info *);
 void pipe_double_lock(struct pipe_inode_info *, struct pipe_inode_info *);
 
 extern unsigned int pipe_max_size, pipe_min_size;
-int pipe_proc_fn(struct ctl_table *, int, void __user *, size_t *, loff_t *);
+int pipe_proc_fn(void);
 
 
 /* Drop the inode semaphore and wait for a pipe event, atomically */
diff --git a/kernel/sysctl.c b/kernel/sysctl.c
index 2104452..35f225b 100644
--- a/kernel/sysctl.c
+++ b/kernel/sysctl.c
@@ -1551,7 +1551,8 @@ static struct ctl_table fs_table[] = {
 		.data		= &pipe_max_size,
 		.maxlen		= sizeof(int),
 		.mode		= 0644,
-		.proc_handler	= &pipe_proc_fn,
+		.proc_handler	= proc_dointvec_minmax,
+		.callback	= &pipe_proc_fn,
 		.extra1		= &pipe_min_size,
 	},
 	{ }
-- 
1.7.8.5

Re: [PATCH 01/14] sysctl: provide callback for write into ctl_table entry

[Eric W. Biederman]

Sasha Levin <levinsasha928@gmail.com> writes:

> Provide a callback that will be called when writing to a ctl_table
> entry after the user input has been validated.
>
> This will simplify user input checks since now it will be possible to
> remove them out of the proc_handler.

Ick No.

You are simplifying things by taking updates out of locks, and
introducing races.

Your naming of the callback "callback" is much too generic.

I think the current function call mechanism of sysctl can be improved
but I don't think you have come up with the right combination of things.

Eric


> Signed-off-by: Sasha Levin <levinsasha928@gmail.com>
> ---
>  fs/proc/proc_sysctl.c  |    4 ++++
>  include/linux/sysctl.h |    1 +
>  2 files changed, 5 insertions(+), 0 deletions(-)
>
> diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c
> index 21d836f..190db28 100644
> --- a/fs/proc/proc_sysctl.c
> +++ b/fs/proc/proc_sysctl.c
> @@ -507,6 +507,10 @@ static ssize_t proc_sys_call_handler(struct file *filp, void __user *buf,
>  	error = table->proc_handler(table, write, buf, &res, ppos);
>  	if (!error)
>  		error = res;
> +
> +	if (!error && write && table->callback)
> +		error = table->callback();
> +
>  out:
>  	sysctl_head_finish(head);
>  
> diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h
> index c34b4c8..27c14cf 100644
> --- a/include/linux/sysctl.h
> +++ b/include/linux/sysctl.h
> @@ -1022,6 +1022,7 @@ struct ctl_table
>  	struct ctl_table_poll *poll;
>  	void *extra1;
>  	void *extra2;
> +	int (*callback)(void);		/* Called when entry is written to */
>  };
>  
>  struct ctl_node {

Re: [PATCH 01/14] sysctl: provide callback for write into ctl_table entry

[Sasha Levin]

On Sun, Apr 29, 2012 at 10:22 AM, Eric W. Biederman
<ebiederm@xmission.com> wrote:
> Sasha Levin <levinsasha928@gmail.com> writes:
>
>> Provide a callback that will be called when writing to a ctl_table
>> entry after the user input has been validated.
>>
>> This will simplify user input checks since now it will be possible to
>> remove them out of the proc_handler.
>
> Ick No.
>
> You are simplifying things by taking updates out of locks, and
> introducing races.

Exactly twp of the patches (out of 14) are taking updates out of
locks. I'm quite sure that doing that in the ftrace case is perfectly
fine, and I'll take a second look at the sched-rt one since there
indeed might be a race caused due to the patch that I've missed.

If we figure out that both cases are wrong, the solution would be to
drop these two patches from the series. I have only simplified that
I've thought to be simple common cases, if I'm mistaken about these
two then they're out.

> Your naming of the callback "callback" is much too generic.

I'd name it write_successful_callback() if there was a point for that,
but seeing as there are no other callback types (and I don't see a
need at the moment for other callbacks either), I've just named it
"callback".

Either way, I'm perfectly fine with renaming it to whatever works for
the rest of the community.

> I think the current function call mechanism of sysctl can be improved
> but I don't think you have come up with the right combination of things.

I'm not trying to fix the entire function call mechanism, I'm just
trying to correct a negative pattern that has developed along time.

If it doesn't fit the bigger view of the future of sysctl function
calls, let me know what's that view is exactly and we'll see if this
patch series can work there. If there's something specific that's
bothering you about this series, let me know and I'll fix it. But
saying that it sucks since it doesn't solve all the issues in sysctl
function calls doesn't work for me.

Re: [PATCH 01/14] sysctl: provide callback for write into ctl_table entry

[Steven Rostedt]

On Sun, 2012-04-29 at 14:07 +0200, Sasha Levin wrote:
> On Sun, Apr 29, 2012 at 10:22 AM, Eric W. Biederman

> Exactly twp of the patches (out of 14) are taking updates out of
> locks. I'm quite sure that doing that in the ftrace case is perfectly
> fine, and I'll take a second look at the sched-rt one since there
> indeed might be a race caused due to the patch that I've missed.

The update of ftrace_enable must be done under the ftrace_lock mutex.
With the exception of ftrace_kill() which is a one shot deal that kills
ftrace updates until a reboot.

-- Steve

Re: [PATCH 01/14] sysctl: provide callback for write into ctl_table entry

[Sasha Levin]

On Sun, Apr 29, 2012 at 4:00 PM, Steven Rostedt <rostedt@goodmis.org> wrote:
> On Sun, 2012-04-29 at 14:07 +0200, Sasha Levin wrote:
>> On Sun, Apr 29, 2012 at 10:22 AM, Eric W. Biederman
>
>> Exactly twp of the patches (out of 14) are taking updates out of
>> locks. I'm quite sure that doing that in the ftrace case is perfectly
>> fine, and I'll take a second look at the sched-rt one since there
>> indeed might be a race caused due to the patch that I've missed.
>
> The update of ftrace_enable must be done under the ftrace_lock mutex.
> With the exception of ftrace_kill() which is a one shot deal that kills
> ftrace updates until a reboot.

Understood.

A fix for that could be having the sysctl modifying a different var,
and having ftrace_enabled from that under a lock, but I'm not sure if
it's worth the work for the cleanup.

Re: [PATCH 01/14] sysctl: provide callback for write into ctl_table entry

[Steven Rostedt]

On Sun, 2012-04-29 at 16:14 +0200, Sasha Levin wrote:

> A fix for that could be having the sysctl modifying a different var,
> and having ftrace_enabled from that under a lock, but I'm not sure if
> it's worth the work for the cleanup.

That was my original plan, but it seemed too much of a hassle than it
was worth, as I needed to make sure the mirrored variable was in sync
with ftrace_enabled, otherwise it could be confusing when ftrace was not
working but sysctl showed ftrace set to 1.

-- Steve

Re: [PATCH 01/14] sysctl: provide callback for write into ctl_table entry

[Eric W. Biederman]

Steven Rostedt <rostedt@goodmis.org> writes:

> On Sun, 2012-04-29 at 16:14 +0200, Sasha Levin wrote:
>
>> A fix for that could be having the sysctl modifying a different var,
>> and having ftrace_enabled from that under a lock, but I'm not sure if
>> it's worth the work for the cleanup.
>
> That was my original plan, but it seemed too much of a hassle than it
> was worth, as I needed to make sure the mirrored variable was in sync
> with ftrace_enabled, otherwise it could be confusing when ftrace was not
> working but sysctl showed ftrace set to 1.

I don't see the problem you are trying to solve with your patches.

What I do see is you have ignored one of the biggest problem with the
current sysctl interface in that it is not easy to plug in your own code
in the cases you need to before an update is made. (Locks permission
checks, etc).

You have also bloated struct ctl_table for no apparent reason.

This current crop of patches was just sloppy.  You showed a poor
choice of function names and did not preserve necessary invariants
when changing the code.  It looks like you exchanged something that
was a bit ugly for something that straight out encourages broken
behavior. 

So I respectfully suggest you go back to the drawing board and figure
out a solution that makes this class of function much easier to write
in a bug free manner.

Eric

Re: [PATCH 01/14] sysctl: provide callback for write into ctl_table entry

[Sasha Levin]

On Sun, Apr 29, 2012 at 8:45 AM, Sasha Levin <levinsasha928@gmail.com> wrote:
> +
> +       if (!error && write && table->callback)
> +               error = table->callback();
> +

Tetsuo Handa has pointed out that 'error' is actually the amount of
bytes read/written in the success case. I'll fix that for V2.