* [PATCH 0/2] sysctl: share commonly used constants @ 2019-04-08 22:09 Matteo Croce 2019-04-08 22:09 ` [PATCH 1/2] proc/sysctl: add shared variables for range check Matteo Croce 2019-04-08 22:09 ` [PATCH 2/2] kernel: use sysctl " Matteo Croce 0 siblings, 2 replies; 14+ messages in thread From: Matteo Croce @ 2019-04-08 22:09 UTC (permalink / raw) To: linux-kernel, linux-fsdevel; +Cc: Luis Chamberlain, Kees Cook, Alexey Dobriyan In the sysctl code there are a lot of duplicate constants used to validate the user input, which could be put in a shared place. Some of them are not even constant, fix this with the following patches. The first one declares the shared constants, the second one makes use of them for the kernel/ sysctls. If merged, I will send separate patches for the other susystems. Matteo Croce (2): proc/sysctl: add shared variables for range check kernel: use sysctl shared variables for range check fs/proc/proc_sysctl.c | 5 ++ include/linux/sysctl.h | 4 + kernel/pid_namespace.c | 3 +- kernel/sysctl.c | 193 ++++++++++++++++++++--------------------- kernel/ucount.c | 6 +- 5 files changed, 107 insertions(+), 104 deletions(-) -- 2.21.0 ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 1/2] proc/sysctl: add shared variables for range check 2019-04-08 22:09 [PATCH 0/2] sysctl: share commonly used constants Matteo Croce @ 2019-04-08 22:09 ` Matteo Croce 2019-04-10 22:18 ` Kees Cook 2019-04-08 22:09 ` [PATCH 2/2] kernel: use sysctl " Matteo Croce 1 sibling, 1 reply; 14+ messages in thread From: Matteo Croce @ 2019-04-08 22:09 UTC (permalink / raw) To: linux-kernel, linux-fsdevel; +Cc: Luis Chamberlain, Kees Cook, Alexey Dobriyan In the sysctl code the proc_dointvec_minmax() function is often used to validate the user supplied value between an allowed range. This function uses the extra1 and extra2 members from struct ctl_table as minimum and maximum allowed value. On sysctl handler declaration, in every source file there are some readonly variables containing just an integer which address is assigned to the extra1 and extra2 members, so the sysctl range is enforced. The special values 0, 1 and INT_MAX are very often used as range boundary, leading duplication of variables like zero=0, one=1, int_max=INT_MAX in different source files: $ git grep -E '\.extra[12].*&(zero|one|int_max)' |wc -l 261 This patch adds three variables for the most commonly used values, so they can be referenced instead of creating a local one for every object file. Signed-off-by: Matteo Croce <mcroce@redhat.com> --- fs/proc/proc_sysctl.c | 5 +++++ include/linux/sysctl.h | 4 ++++ 2 files changed, 9 insertions(+) diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c index d65390727541..e7a96169fced 100644 --- a/fs/proc/proc_sysctl.c +++ b/fs/proc/proc_sysctl.c @@ -21,6 +21,11 @@ static const struct inode_operations proc_sys_inode_operations; static const struct file_operations proc_sys_dir_file_operations; static const struct inode_operations proc_sys_dir_operations; +/* shared constants to be used in various sysctls */ +const int sysctl_zero = 0; +const int sysctl_one = 1; +const int sysctl_int_max = INT_MAX; + /* Support for permanently empty directories */ struct ctl_table sysctl_mount_point[] = { diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h index b769ecfcc3bd..f3b191799747 100644 --- a/include/linux/sysctl.h +++ b/include/linux/sysctl.h @@ -37,6 +37,10 @@ struct ctl_table_root; struct ctl_table_header; struct ctl_dir; +extern const int sysctl_zero; +extern const int sysctl_one; +extern const int sysctl_int_max; + typedef int proc_handler (struct ctl_table *ctl, int write, void __user *buffer, size_t *lenp, loff_t *ppos); -- 2.21.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 1/2] proc/sysctl: add shared variables for range check 2019-04-08 22:09 ` [PATCH 1/2] proc/sysctl: add shared variables for range check Matteo Croce @ 2019-04-10 22:18 ` Kees Cook 0 siblings, 0 replies; 14+ messages in thread From: Kees Cook @ 2019-04-10 22:18 UTC (permalink / raw) To: Matteo Croce Cc: LKML, linux-fsdevel, Luis Chamberlain, Kees Cook, Alexey Dobriyan On Mon, Apr 8, 2019 at 3:09 PM Matteo Croce <mcroce@redhat.com> wrote: > > In the sysctl code the proc_dointvec_minmax() function is often used to > validate the user supplied value between an allowed range. This function > uses the extra1 and extra2 members from struct ctl_table as minimum and > maximum allowed value. > > On sysctl handler declaration, in every source file there are some readonly > variables containing just an integer which address is assigned to the > extra1 and extra2 members, so the sysctl range is enforced. > > The special values 0, 1 and INT_MAX are very often used as range boundary, > leading duplication of variables like zero=0, one=1, int_max=INT_MAX in > different source files: > > $ git grep -E '\.extra[12].*&(zero|one|int_max)' |wc -l > 261 > > This patch adds three variables for the most commonly used values, so they > can be referenced instead of creating a local one for every object file. > > Signed-off-by: Matteo Croce <mcroce@redhat.com> Oh, and for this patch: Yes please; let's get everyone to use these instead of littering the kernel with (non-const!) copies of the same thing. :) Acked-by: Kees Cook <keescook@chromium.org> -Kees > --- > fs/proc/proc_sysctl.c | 5 +++++ > include/linux/sysctl.h | 4 ++++ > 2 files changed, 9 insertions(+) > > diff --git a/fs/proc/proc_sysctl.c b/fs/proc/proc_sysctl.c > index d65390727541..e7a96169fced 100644 > --- a/fs/proc/proc_sysctl.c > +++ b/fs/proc/proc_sysctl.c > @@ -21,6 +21,11 @@ static const struct inode_operations proc_sys_inode_operations; > static const struct file_operations proc_sys_dir_file_operations; > static const struct inode_operations proc_sys_dir_operations; > > +/* shared constants to be used in various sysctls */ > +const int sysctl_zero = 0; > +const int sysctl_one = 1; > +const int sysctl_int_max = INT_MAX; > + > /* Support for permanently empty directories */ > > struct ctl_table sysctl_mount_point[] = { > diff --git a/include/linux/sysctl.h b/include/linux/sysctl.h > index b769ecfcc3bd..f3b191799747 100644 > --- a/include/linux/sysctl.h > +++ b/include/linux/sysctl.h > @@ -37,6 +37,10 @@ struct ctl_table_root; > struct ctl_table_header; > struct ctl_dir; > > +extern const int sysctl_zero; > +extern const int sysctl_one; > +extern const int sysctl_int_max; > + > typedef int proc_handler (struct ctl_table *ctl, int write, > void __user *buffer, size_t *lenp, loff_t *ppos); > > -- > 2.21.0 > -- Kees Cook ^ permalink raw reply [flat|nested] 14+ messages in thread
* [PATCH 2/2] kernel: use sysctl shared variables for range check 2019-04-08 22:09 [PATCH 0/2] sysctl: share commonly used constants Matteo Croce 2019-04-08 22:09 ` [PATCH 1/2] proc/sysctl: add shared variables for range check Matteo Croce @ 2019-04-08 22:09 ` Matteo Croce 2019-04-10 18:46 ` Kees Cook 2019-04-17 3:21 ` Kees Cook 1 sibling, 2 replies; 14+ messages in thread From: Matteo Croce @ 2019-04-08 22:09 UTC (permalink / raw) To: linux-kernel, linux-fsdevel; +Cc: Luis Chamberlain, Kees Cook, Alexey Dobriyan Use the shared variables for range check, instead of declaring a local one in every source file. Signed-off-by: Matteo Croce <mcroce@redhat.com> --- kernel/pid_namespace.c | 3 +- kernel/sysctl.c | 193 ++++++++++++++++++++--------------------- kernel/ucount.c | 6 +- 3 files changed, 98 insertions(+), 104 deletions(-) diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c index aa6e72fb7c08..ddbb51bc4968 100644 --- a/kernel/pid_namespace.c +++ b/kernel/pid_namespace.c @@ -290,14 +290,13 @@ static int pid_ns_ctl_handler(struct ctl_table *table, int write, } extern int pid_max; -static int zero = 0; static struct ctl_table pid_ns_ctl_table[] = { { .procname = "ns_last_pid", .maxlen = sizeof(int), .mode = 0666, /* permissions are checked in the handler */ .proc_handler = pid_ns_ctl_handler, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &pid_max, }, { } diff --git a/kernel/sysctl.c b/kernel/sysctl.c index 553b19439714..d6f4b26951e1 100644 --- a/kernel/sysctl.c +++ b/kernel/sysctl.c @@ -123,9 +123,6 @@ static int sixty = 60; #endif static int __maybe_unused neg_one = -1; - -static int zero; -static int __maybe_unused one = 1; static int __maybe_unused two = 2; static int __maybe_unused four = 4; static unsigned long zero_ul; @@ -388,8 +385,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(unsigned int), .mode = 0644, .proc_handler = sysctl_schedstats, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #endif /* CONFIG_SCHEDSTATS */ #endif /* CONFIG_SMP */ @@ -421,7 +418,7 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(unsigned int), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &one, + .extra1 = (void *)&sysctl_one, }, { .procname = "numa_balancing", @@ -429,8 +426,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(unsigned int), .mode = 0644, .proc_handler = sysctl_numa_balancing, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #endif /* CONFIG_NUMA_BALANCING */ #endif /* CONFIG_SCHED_DEBUG */ @@ -462,8 +459,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(unsigned int), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #endif #ifdef CONFIG_CFS_BANDWIDTH @@ -473,7 +470,7 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(unsigned int), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &one, + .extra1 = (void *)&sysctl_one, }, #endif #if defined(CONFIG_ENERGY_MODEL) && defined(CONFIG_CPU_FREQ_GOV_SCHEDUTIL) @@ -483,8 +480,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(unsigned int), .mode = 0644, .proc_handler = sched_energy_aware_handler, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #endif #ifdef CONFIG_PROVE_LOCKING @@ -549,7 +546,7 @@ static struct ctl_table kern_table[] = { .mode = 0644, .proc_handler = proc_dointvec_minmax, .extra1 = &neg_one, - .extra2 = &one, + .extra2 = (void *)&sysctl_one, }, #endif #ifdef CONFIG_LATENCYTOP @@ -683,8 +680,8 @@ static struct ctl_table kern_table[] = { .mode = 0644, /* only handle a transition from default "0" to "1" */ .proc_handler = proc_dointvec_minmax, - .extra1 = &one, - .extra2 = &one, + .extra1 = (void *)&sysctl_one, + .extra2 = (void *)&sysctl_one, }, #endif #ifdef CONFIG_MODULES @@ -702,8 +699,8 @@ static struct ctl_table kern_table[] = { .mode = 0644, /* only handle a transition from default "0" to "1" */ .proc_handler = proc_dointvec_minmax, - .extra1 = &one, - .extra2 = &one, + .extra1 = (void *)&sysctl_one, + .extra2 = (void *)&sysctl_one, }, #endif #ifdef CONFIG_UEVENT_HELPER @@ -862,7 +859,7 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &ten_thousand, }, { @@ -878,8 +875,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax_sysadmin, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, { .procname = "kptr_restrict", @@ -887,7 +884,7 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax_sysadmin, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &two, }, #endif @@ -912,8 +909,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_watchdog, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, { .procname = "watchdog_thresh", @@ -921,7 +918,7 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_watchdog_thresh, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &sixty, }, { @@ -930,8 +927,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(int), .mode = NMI_WATCHDOG_SYSCTL_PERM, .proc_handler = proc_nmi_watchdog, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, { .procname = "watchdog_cpumask", @@ -947,8 +944,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_soft_watchdog, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, { .procname = "softlockup_panic", @@ -956,8 +953,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #ifdef CONFIG_SMP { @@ -966,8 +963,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #endif /* CONFIG_SMP */ #endif @@ -978,8 +975,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #ifdef CONFIG_SMP { @@ -988,8 +985,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #endif /* CONFIG_SMP */ #endif @@ -1102,8 +1099,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, { .procname = "hung_task_check_count", @@ -1111,7 +1108,7 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, }, { .procname = "hung_task_timeout_secs", @@ -1188,7 +1185,7 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(sysctl_perf_event_sample_rate), .mode = 0644, .proc_handler = perf_proc_update_handler, - .extra1 = &one, + .extra1 = (void *)&sysctl_one, }, { .procname = "perf_cpu_time_max_percent", @@ -1196,7 +1193,7 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(sysctl_perf_cpu_time_max_percent), .mode = 0644, .proc_handler = perf_cpu_time_max_percent_handler, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &one_hundred, }, { @@ -1205,7 +1202,7 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(sysctl_perf_event_max_stack), .mode = 0644, .proc_handler = perf_event_max_stack_handler, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &six_hundred_forty_kb, }, { @@ -1214,7 +1211,7 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(sysctl_perf_event_max_contexts_per_stack), .mode = 0644, .proc_handler = perf_event_max_stack_handler, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &one_thousand, }, #endif @@ -1224,8 +1221,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON) { @@ -1234,8 +1231,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(unsigned int), .mode = 0644, .proc_handler = timer_migration_handler, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #endif #ifdef CONFIG_BPF_SYSCALL @@ -1246,8 +1243,8 @@ static struct ctl_table kern_table[] = { .mode = 0644, /* only handle a transition from default "0" to "1" */ .proc_handler = proc_dointvec_minmax, - .extra1 = &one, - .extra2 = &one, + .extra1 = (void *)&sysctl_one, + .extra2 = (void *)&sysctl_one, }, { .procname = "bpf_stats_enabled", @@ -1255,8 +1252,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(sysctl_bpf_stats_enabled), .mode = 0644, .proc_handler = proc_dointvec_minmax_bpf_stats, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #endif #if defined(CONFIG_TREE_RCU) || defined(CONFIG_PREEMPT_RCU) @@ -1266,8 +1263,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(sysctl_panic_on_rcu_stall), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #endif #ifdef CONFIG_STACKLEAK_RUNTIME_DISABLE @@ -1277,8 +1274,8 @@ static struct ctl_table kern_table[] = { .maxlen = sizeof(int), .mode = 0600, .proc_handler = stack_erasing_sysctl, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #endif { } @@ -1291,7 +1288,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(sysctl_overcommit_memory), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &two, }, { @@ -1300,7 +1297,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(sysctl_panic_on_oom), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &two, }, { @@ -1337,7 +1334,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, }, { .procname = "dirty_background_ratio", @@ -1345,7 +1342,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(dirty_background_ratio), .mode = 0644, .proc_handler = dirty_background_ratio_handler, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &one_hundred, }, { @@ -1362,7 +1359,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(vm_dirty_ratio), .mode = 0644, .proc_handler = dirty_ratio_handler, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &one_hundred, }, { @@ -1386,7 +1383,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(dirty_expire_interval), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, }, { .procname = "dirtytime_expire_seconds", @@ -1394,7 +1391,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(dirtytime_expire_interval), .mode = 0644, .proc_handler = dirtytime_interval_handler, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, }, { .procname = "swappiness", @@ -1402,7 +1399,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(vm_swappiness), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &one_hundred, }, #ifdef CONFIG_HUGETLB_PAGE @@ -1427,8 +1424,8 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = sysctl_vm_numa_stat_handler, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #endif { @@ -1459,7 +1456,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = drop_caches_sysctl_handler, - .extra1 = &one, + .extra1 = (void *)&sysctl_one, .extra2 = &four, }, #ifdef CONFIG_COMPACTION @@ -1485,8 +1482,8 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #endif /* CONFIG_COMPACTION */ @@ -1496,7 +1493,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(min_free_kbytes), .mode = 0644, .proc_handler = min_free_kbytes_sysctl_handler, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, }, { .procname = "watermark_boost_factor", @@ -1504,7 +1501,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(watermark_boost_factor), .mode = 0644, .proc_handler = watermark_boost_factor_sysctl_handler, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, }, { .procname = "watermark_scale_factor", @@ -1512,7 +1509,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(watermark_scale_factor), .mode = 0644, .proc_handler = watermark_scale_factor_sysctl_handler, - .extra1 = &one, + .extra1 = (void *)&sysctl_one, .extra2 = &one_thousand, }, { @@ -1521,7 +1518,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(percpu_pagelist_fraction), .mode = 0644, .proc_handler = percpu_pagelist_fraction_sysctl_handler, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, }, #ifdef CONFIG_MMU { @@ -1530,7 +1527,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(sysctl_max_map_count), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, }, #else { @@ -1539,7 +1536,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(sysctl_nr_trim_pages), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, }, #endif { @@ -1555,7 +1552,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(block_dump), .mode = 0644, .proc_handler = proc_dointvec, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, }, { .procname = "vfs_cache_pressure", @@ -1563,7 +1560,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(sysctl_vfs_cache_pressure), .mode = 0644, .proc_handler = proc_dointvec, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, }, #ifdef HAVE_ARCH_PICK_MMAP_LAYOUT { @@ -1572,7 +1569,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(sysctl_legacy_va_layout), .mode = 0644, .proc_handler = proc_dointvec, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, }, #endif #ifdef CONFIG_NUMA @@ -1582,7 +1579,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(node_reclaim_mode), .mode = 0644, .proc_handler = proc_dointvec, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, }, { .procname = "min_unmapped_ratio", @@ -1590,7 +1587,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(sysctl_min_unmapped_ratio), .mode = 0644, .proc_handler = sysctl_min_unmapped_ratio_sysctl_handler, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &one_hundred, }, { @@ -1599,7 +1596,7 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(sysctl_min_slab_ratio), .mode = 0644, .proc_handler = sysctl_min_slab_ratio_sysctl_handler, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &one_hundred, }, #endif @@ -1650,7 +1647,7 @@ static struct ctl_table vm_table[] = { #endif .mode = 0644, .proc_handler = proc_dointvec, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, }, #endif #ifdef CONFIG_HIGHMEM @@ -1660,8 +1657,8 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(vm_highmem_is_dirtyable), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #endif #ifdef CONFIG_MEMORY_FAILURE @@ -1671,8 +1668,8 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(sysctl_memory_failure_early_kill), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, { .procname = "memory_failure_recovery", @@ -1680,8 +1677,8 @@ static struct ctl_table vm_table[] = { .maxlen = sizeof(sysctl_memory_failure_recovery), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #endif { @@ -1853,8 +1850,8 @@ static struct ctl_table fs_table[] = { .maxlen = sizeof(int), .mode = 0600, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, { .procname = "protected_hardlinks", @@ -1862,8 +1859,8 @@ static struct ctl_table fs_table[] = { .maxlen = sizeof(int), .mode = 0600, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, { .procname = "protected_fifos", @@ -1871,7 +1868,7 @@ static struct ctl_table fs_table[] = { .maxlen = sizeof(int), .mode = 0600, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &two, }, { @@ -1880,7 +1877,7 @@ static struct ctl_table fs_table[] = { .maxlen = sizeof(int), .mode = 0600, .proc_handler = proc_dointvec_minmax, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &two, }, { @@ -1889,7 +1886,7 @@ static struct ctl_table fs_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_dointvec_minmax_coredump, - .extra1 = &zero, + .extra1 = (void *)&sysctl_zero, .extra2 = &two, }, #if defined(CONFIG_BINFMT_MISC) || defined(CONFIG_BINFMT_MISC_MODULE) @@ -1926,7 +1923,7 @@ static struct ctl_table fs_table[] = { .maxlen = sizeof(unsigned int), .mode = 0644, .proc_handler = proc_dointvec_minmax, - .extra1 = &one, + .extra1 = (void *)&sysctl_one, }, { } }; @@ -1948,8 +1945,8 @@ static struct ctl_table debug_table[] = { .maxlen = sizeof(int), .mode = 0644, .proc_handler = proc_kprobes_optimization_handler, - .extra1 = &zero, - .extra2 = &one, + .extra1 = (void *)&sysctl_zero, + .extra2 = (void *)&sysctl_one, }, #endif { } diff --git a/kernel/ucount.c b/kernel/ucount.c index f48d1b6376a4..ba7b8282d299 100644 --- a/kernel/ucount.c +++ b/kernel/ucount.c @@ -57,16 +57,14 @@ static struct ctl_table_root set_root = { .permissions = set_permissions, }; -static int zero = 0; -static int int_max = INT_MAX; #define UCOUNT_ENTRY(name) \ { \ .procname = name, \ .maxlen = sizeof(int), \ .mode = 0644, \ .proc_handler = proc_dointvec_minmax, \ - .extra1 = &zero, \ - .extra2 = &int_max, \ + .extra1 = (void *)&sysctl_zero, \ + .extra2 = (void *)&sysctl_int_max, \ } static struct ctl_table user_table[] = { UCOUNT_ENTRY("max_user_namespaces"), -- 2.21.0 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] kernel: use sysctl shared variables for range check 2019-04-08 22:09 ` [PATCH 2/2] kernel: use sysctl " Matteo Croce @ 2019-04-10 18:46 ` Kees Cook 2019-04-10 19:23 ` Matteo Croce 2019-04-17 3:21 ` Kees Cook 1 sibling, 1 reply; 14+ messages in thread From: Kees Cook @ 2019-04-10 18:46 UTC (permalink / raw) To: Matteo Croce Cc: LKML, linux-fsdevel, Luis Chamberlain, Kees Cook, Alexey Dobriyan On Mon, Apr 8, 2019 at 3:09 PM Matteo Croce <mcroce@redhat.com> wrote: > > Use the shared variables for range check, instead of declaring a local one > in every source file. I was expecting this to be a tree-wide change for all the cases found by patch 1's "git grep". Slight change to the grep for higher accuracy: $ git grep -E '\.extra[12].*&(zero|one|int_max)\b' |wc -l 245 Only 31 sources: $ git grep -E '\.extra[12].*&(zero|one|int_max)\b' | cut -d: -f1 | sort -u > /tmp/list.txt $ wc -l /tmp/list.txt 31 One thing I wonder about is if any of these cases depend on the extra variable being non-const (many of these are just "static int"). $ egrep -H '\b(zero|one|int_max)\b.*=' $(cat /tmp/list.txt) | grep -v static Looks like none, so it'd be safe. How about doing this tree-wide for all 31 cases? (Coccinelle might be able to help.) -Kees > > Signed-off-by: Matteo Croce <mcroce@redhat.com> > --- > kernel/pid_namespace.c | 3 +- > kernel/sysctl.c | 193 ++++++++++++++++++++--------------------- > kernel/ucount.c | 6 +- > 3 files changed, 98 insertions(+), 104 deletions(-) > > diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c > index aa6e72fb7c08..ddbb51bc4968 100644 > --- a/kernel/pid_namespace.c > +++ b/kernel/pid_namespace.c > @@ -290,14 +290,13 @@ static int pid_ns_ctl_handler(struct ctl_table *table, int write, > } > > extern int pid_max; > -static int zero = 0; > static struct ctl_table pid_ns_ctl_table[] = { > { > .procname = "ns_last_pid", > .maxlen = sizeof(int), > .mode = 0666, /* permissions are checked in the handler */ > .proc_handler = pid_ns_ctl_handler, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &pid_max, > }, > { } > diff --git a/kernel/sysctl.c b/kernel/sysctl.c > index 553b19439714..d6f4b26951e1 100644 > --- a/kernel/sysctl.c > +++ b/kernel/sysctl.c > @@ -123,9 +123,6 @@ static int sixty = 60; > #endif > > static int __maybe_unused neg_one = -1; > - > -static int zero; > -static int __maybe_unused one = 1; > static int __maybe_unused two = 2; > static int __maybe_unused four = 4; > static unsigned long zero_ul; > @@ -388,8 +385,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(unsigned int), > .mode = 0644, > .proc_handler = sysctl_schedstats, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #endif /* CONFIG_SCHEDSTATS */ > #endif /* CONFIG_SMP */ > @@ -421,7 +418,7 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(unsigned int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &one, > + .extra1 = (void *)&sysctl_one, > }, > { > .procname = "numa_balancing", > @@ -429,8 +426,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(unsigned int), > .mode = 0644, > .proc_handler = sysctl_numa_balancing, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #endif /* CONFIG_NUMA_BALANCING */ > #endif /* CONFIG_SCHED_DEBUG */ > @@ -462,8 +459,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(unsigned int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #endif > #ifdef CONFIG_CFS_BANDWIDTH > @@ -473,7 +470,7 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(unsigned int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &one, > + .extra1 = (void *)&sysctl_one, > }, > #endif > #if defined(CONFIG_ENERGY_MODEL) && defined(CONFIG_CPU_FREQ_GOV_SCHEDUTIL) > @@ -483,8 +480,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(unsigned int), > .mode = 0644, > .proc_handler = sched_energy_aware_handler, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #endif > #ifdef CONFIG_PROVE_LOCKING > @@ -549,7 +546,7 @@ static struct ctl_table kern_table[] = { > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > .extra1 = &neg_one, > - .extra2 = &one, > + .extra2 = (void *)&sysctl_one, > }, > #endif > #ifdef CONFIG_LATENCYTOP > @@ -683,8 +680,8 @@ static struct ctl_table kern_table[] = { > .mode = 0644, > /* only handle a transition from default "0" to "1" */ > .proc_handler = proc_dointvec_minmax, > - .extra1 = &one, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_one, > + .extra2 = (void *)&sysctl_one, > }, > #endif > #ifdef CONFIG_MODULES > @@ -702,8 +699,8 @@ static struct ctl_table kern_table[] = { > .mode = 0644, > /* only handle a transition from default "0" to "1" */ > .proc_handler = proc_dointvec_minmax, > - .extra1 = &one, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_one, > + .extra2 = (void *)&sysctl_one, > }, > #endif > #ifdef CONFIG_UEVENT_HELPER > @@ -862,7 +859,7 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &ten_thousand, > }, > { > @@ -878,8 +875,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax_sysadmin, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > { > .procname = "kptr_restrict", > @@ -887,7 +884,7 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax_sysadmin, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &two, > }, > #endif > @@ -912,8 +909,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_watchdog, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > { > .procname = "watchdog_thresh", > @@ -921,7 +918,7 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_watchdog_thresh, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &sixty, > }, > { > @@ -930,8 +927,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(int), > .mode = NMI_WATCHDOG_SYSCTL_PERM, > .proc_handler = proc_nmi_watchdog, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > { > .procname = "watchdog_cpumask", > @@ -947,8 +944,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_soft_watchdog, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > { > .procname = "softlockup_panic", > @@ -956,8 +953,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #ifdef CONFIG_SMP > { > @@ -966,8 +963,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #endif /* CONFIG_SMP */ > #endif > @@ -978,8 +975,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #ifdef CONFIG_SMP > { > @@ -988,8 +985,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #endif /* CONFIG_SMP */ > #endif > @@ -1102,8 +1099,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > { > .procname = "hung_task_check_count", > @@ -1111,7 +1108,7 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > }, > { > .procname = "hung_task_timeout_secs", > @@ -1188,7 +1185,7 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(sysctl_perf_event_sample_rate), > .mode = 0644, > .proc_handler = perf_proc_update_handler, > - .extra1 = &one, > + .extra1 = (void *)&sysctl_one, > }, > { > .procname = "perf_cpu_time_max_percent", > @@ -1196,7 +1193,7 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(sysctl_perf_cpu_time_max_percent), > .mode = 0644, > .proc_handler = perf_cpu_time_max_percent_handler, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &one_hundred, > }, > { > @@ -1205,7 +1202,7 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(sysctl_perf_event_max_stack), > .mode = 0644, > .proc_handler = perf_event_max_stack_handler, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &six_hundred_forty_kb, > }, > { > @@ -1214,7 +1211,7 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(sysctl_perf_event_max_contexts_per_stack), > .mode = 0644, > .proc_handler = perf_event_max_stack_handler, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &one_thousand, > }, > #endif > @@ -1224,8 +1221,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #if defined(CONFIG_SMP) && defined(CONFIG_NO_HZ_COMMON) > { > @@ -1234,8 +1231,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(unsigned int), > .mode = 0644, > .proc_handler = timer_migration_handler, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #endif > #ifdef CONFIG_BPF_SYSCALL > @@ -1246,8 +1243,8 @@ static struct ctl_table kern_table[] = { > .mode = 0644, > /* only handle a transition from default "0" to "1" */ > .proc_handler = proc_dointvec_minmax, > - .extra1 = &one, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_one, > + .extra2 = (void *)&sysctl_one, > }, > { > .procname = "bpf_stats_enabled", > @@ -1255,8 +1252,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(sysctl_bpf_stats_enabled), > .mode = 0644, > .proc_handler = proc_dointvec_minmax_bpf_stats, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #endif > #if defined(CONFIG_TREE_RCU) || defined(CONFIG_PREEMPT_RCU) > @@ -1266,8 +1263,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(sysctl_panic_on_rcu_stall), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #endif > #ifdef CONFIG_STACKLEAK_RUNTIME_DISABLE > @@ -1277,8 +1274,8 @@ static struct ctl_table kern_table[] = { > .maxlen = sizeof(int), > .mode = 0600, > .proc_handler = stack_erasing_sysctl, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #endif > { } > @@ -1291,7 +1288,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(sysctl_overcommit_memory), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &two, > }, > { > @@ -1300,7 +1297,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(sysctl_panic_on_oom), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &two, > }, > { > @@ -1337,7 +1334,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > }, > { > .procname = "dirty_background_ratio", > @@ -1345,7 +1342,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(dirty_background_ratio), > .mode = 0644, > .proc_handler = dirty_background_ratio_handler, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &one_hundred, > }, > { > @@ -1362,7 +1359,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(vm_dirty_ratio), > .mode = 0644, > .proc_handler = dirty_ratio_handler, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &one_hundred, > }, > { > @@ -1386,7 +1383,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(dirty_expire_interval), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > }, > { > .procname = "dirtytime_expire_seconds", > @@ -1394,7 +1391,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(dirtytime_expire_interval), > .mode = 0644, > .proc_handler = dirtytime_interval_handler, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > }, > { > .procname = "swappiness", > @@ -1402,7 +1399,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(vm_swappiness), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &one_hundred, > }, > #ifdef CONFIG_HUGETLB_PAGE > @@ -1427,8 +1424,8 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = sysctl_vm_numa_stat_handler, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #endif > { > @@ -1459,7 +1456,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = drop_caches_sysctl_handler, > - .extra1 = &one, > + .extra1 = (void *)&sysctl_one, > .extra2 = &four, > }, > #ifdef CONFIG_COMPACTION > @@ -1485,8 +1482,8 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_dointvec, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > > #endif /* CONFIG_COMPACTION */ > @@ -1496,7 +1493,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(min_free_kbytes), > .mode = 0644, > .proc_handler = min_free_kbytes_sysctl_handler, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > }, > { > .procname = "watermark_boost_factor", > @@ -1504,7 +1501,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(watermark_boost_factor), > .mode = 0644, > .proc_handler = watermark_boost_factor_sysctl_handler, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > }, > { > .procname = "watermark_scale_factor", > @@ -1512,7 +1509,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(watermark_scale_factor), > .mode = 0644, > .proc_handler = watermark_scale_factor_sysctl_handler, > - .extra1 = &one, > + .extra1 = (void *)&sysctl_one, > .extra2 = &one_thousand, > }, > { > @@ -1521,7 +1518,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(percpu_pagelist_fraction), > .mode = 0644, > .proc_handler = percpu_pagelist_fraction_sysctl_handler, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > }, > #ifdef CONFIG_MMU > { > @@ -1530,7 +1527,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(sysctl_max_map_count), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > }, > #else > { > @@ -1539,7 +1536,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(sysctl_nr_trim_pages), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > }, > #endif > { > @@ -1555,7 +1552,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(block_dump), > .mode = 0644, > .proc_handler = proc_dointvec, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > }, > { > .procname = "vfs_cache_pressure", > @@ -1563,7 +1560,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(sysctl_vfs_cache_pressure), > .mode = 0644, > .proc_handler = proc_dointvec, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > }, > #ifdef HAVE_ARCH_PICK_MMAP_LAYOUT > { > @@ -1572,7 +1569,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(sysctl_legacy_va_layout), > .mode = 0644, > .proc_handler = proc_dointvec, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > }, > #endif > #ifdef CONFIG_NUMA > @@ -1582,7 +1579,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(node_reclaim_mode), > .mode = 0644, > .proc_handler = proc_dointvec, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > }, > { > .procname = "min_unmapped_ratio", > @@ -1590,7 +1587,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(sysctl_min_unmapped_ratio), > .mode = 0644, > .proc_handler = sysctl_min_unmapped_ratio_sysctl_handler, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &one_hundred, > }, > { > @@ -1599,7 +1596,7 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(sysctl_min_slab_ratio), > .mode = 0644, > .proc_handler = sysctl_min_slab_ratio_sysctl_handler, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &one_hundred, > }, > #endif > @@ -1650,7 +1647,7 @@ static struct ctl_table vm_table[] = { > #endif > .mode = 0644, > .proc_handler = proc_dointvec, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > }, > #endif > #ifdef CONFIG_HIGHMEM > @@ -1660,8 +1657,8 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(vm_highmem_is_dirtyable), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #endif > #ifdef CONFIG_MEMORY_FAILURE > @@ -1671,8 +1668,8 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(sysctl_memory_failure_early_kill), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > { > .procname = "memory_failure_recovery", > @@ -1680,8 +1677,8 @@ static struct ctl_table vm_table[] = { > .maxlen = sizeof(sysctl_memory_failure_recovery), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #endif > { > @@ -1853,8 +1850,8 @@ static struct ctl_table fs_table[] = { > .maxlen = sizeof(int), > .mode = 0600, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > { > .procname = "protected_hardlinks", > @@ -1862,8 +1859,8 @@ static struct ctl_table fs_table[] = { > .maxlen = sizeof(int), > .mode = 0600, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > { > .procname = "protected_fifos", > @@ -1871,7 +1868,7 @@ static struct ctl_table fs_table[] = { > .maxlen = sizeof(int), > .mode = 0600, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &two, > }, > { > @@ -1880,7 +1877,7 @@ static struct ctl_table fs_table[] = { > .maxlen = sizeof(int), > .mode = 0600, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &two, > }, > { > @@ -1889,7 +1886,7 @@ static struct ctl_table fs_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax_coredump, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, > .extra2 = &two, > }, > #if defined(CONFIG_BINFMT_MISC) || defined(CONFIG_BINFMT_MISC_MODULE) > @@ -1926,7 +1923,7 @@ static struct ctl_table fs_table[] = { > .maxlen = sizeof(unsigned int), > .mode = 0644, > .proc_handler = proc_dointvec_minmax, > - .extra1 = &one, > + .extra1 = (void *)&sysctl_one, > }, > { } > }; > @@ -1948,8 +1945,8 @@ static struct ctl_table debug_table[] = { > .maxlen = sizeof(int), > .mode = 0644, > .proc_handler = proc_kprobes_optimization_handler, > - .extra1 = &zero, > - .extra2 = &one, > + .extra1 = (void *)&sysctl_zero, > + .extra2 = (void *)&sysctl_one, > }, > #endif > { } > diff --git a/kernel/ucount.c b/kernel/ucount.c > index f48d1b6376a4..ba7b8282d299 100644 > --- a/kernel/ucount.c > +++ b/kernel/ucount.c > @@ -57,16 +57,14 @@ static struct ctl_table_root set_root = { > .permissions = set_permissions, > }; > > -static int zero = 0; > -static int int_max = INT_MAX; > #define UCOUNT_ENTRY(name) \ > { \ > .procname = name, \ > .maxlen = sizeof(int), \ > .mode = 0644, \ > .proc_handler = proc_dointvec_minmax, \ > - .extra1 = &zero, \ > - .extra2 = &int_max, \ > + .extra1 = (void *)&sysctl_zero, \ > + .extra2 = (void *)&sysctl_int_max, \ > } > static struct ctl_table user_table[] = { > UCOUNT_ENTRY("max_user_namespaces"), > -- > 2.21.0 > -- Kees Cook ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] kernel: use sysctl shared variables for range check 2019-04-10 18:46 ` Kees Cook @ 2019-04-10 19:23 ` Matteo Croce 2019-04-10 21:50 ` Kees Cook 0 siblings, 1 reply; 14+ messages in thread From: Matteo Croce @ 2019-04-10 19:23 UTC (permalink / raw) To: Kees Cook; +Cc: LKML, linux-fsdevel, Luis Chamberlain, Alexey Dobriyan On Wed, Apr 10, 2019 at 8:46 PM Kees Cook <keescook@chromium.org> wrote: > > On Mon, Apr 8, 2019 at 3:09 PM Matteo Croce <mcroce@redhat.com> wrote: > > > > Use the shared variables for range check, instead of declaring a local one > > in every source file. > > I was expecting this to be a tree-wide change for all the cases found > by patch 1's "git grep". > Hi Kees, I have already the whole patch ready, but I was frightened by the output of get_maintainer.pl, so I decided to split the patch into small pieces and send the first one. Patches for /proc/sys/net and drivers/ are pretty big, and can be merged after the 1/2 inclusion. > Slight change to the grep for higher accuracy: > > $ git grep -E '\.extra[12].*&(zero|one|int_max)\b' |wc -l > 245 > Right, my regexp wrongly matches also one_hundred, one_jiffy, etc. Anywqay, I did the changes by hand, so apart the commit message, the content should be safe. > Only 31 sources: > $ git grep -E '\.extra[12].*&(zero|one|int_max)\b' | cut -d: -f1 | > sort -u > /tmp/list.txt > $ wc -l /tmp/list.txt > 31 > > One thing I wonder about is if any of these cases depend on the extra > variable being non-const (many of these are just "static int"). > > $ egrep -H '\b(zero|one|int_max)\b.*=' $(cat /tmp/list.txt) | grep -v static > > Looks like none, so it'd be safe. How about doing this tree-wide for > all 31 cases? (Coccinelle might be able to help.) > It could be true for other sysctl values like xpc_disengage_max_timelimit or fscache_op_wq, but it's very unlikely that someone writes, for example, 5 into a variable named "zero". If it does, it most likely a bug, so const is our friend. Regards, -- Matteo Croce per aspera ad upstream ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] kernel: use sysctl shared variables for range check 2019-04-10 19:23 ` Matteo Croce @ 2019-04-10 21:50 ` Kees Cook 2019-04-10 22:30 ` Matteo Croce 0 siblings, 1 reply; 14+ messages in thread From: Kees Cook @ 2019-04-10 21:50 UTC (permalink / raw) To: Matteo Croce, Andrew Morton Cc: LKML, linux-fsdevel, Luis Chamberlain, Alexey Dobriyan On Wed, Apr 10, 2019 at 12:24 PM Matteo Croce <mcroce@redhat.com> wrote: > > On Wed, Apr 10, 2019 at 8:46 PM Kees Cook <keescook@chromium.org> wrote: > > > > On Mon, Apr 8, 2019 at 3:09 PM Matteo Croce <mcroce@redhat.com> wrote: > > > > > > Use the shared variables for range check, instead of declaring a local one > > > in every source file. > > > > I was expecting this to be a tree-wide change for all the cases found > > by patch 1's "git grep". > > > > Hi Kees, > > I have already the whole patch ready, but I was frightened by the > output of get_maintainer.pl, so I decided to split the patch into > small pieces and send the first one. Heh, sounds fine. Normally the big tree-wide changes go via Linus just before cutting rc1 (or rc2). This is "only" 31 source files, though, so maybe akpm wants to take these instead? Andrew, how do you feel about that? > Patches for /proc/sys/net and drivers/ are pretty big, and can be > merged after the 1/2 inclusion. > > > Slight change to the grep for higher accuracy: > > > > $ git grep -E '\.extra[12].*&(zero|one|int_max)\b' |wc -l > > 245 > > > > Right, my regexp wrongly matches also one_hundred, one_jiffy, etc. > Anywqay, I did the changes by hand, so apart the commit message, the > content should be safe. > > > Only 31 sources: > > $ git grep -E '\.extra[12].*&(zero|one|int_max)\b' | cut -d: -f1 | > > sort -u > /tmp/list.txt > > $ wc -l /tmp/list.txt > > 31 > > > > One thing I wonder about is if any of these cases depend on the extra > > variable being non-const (many of these are just "static int"). > > > > $ egrep -H '\b(zero|one|int_max)\b.*=' $(cat /tmp/list.txt) | grep -v static > > > > Looks like none, so it'd be safe. How about doing this tree-wide for > > all 31 cases? (Coccinelle might be able to help.) > > It could be true for other sysctl values like > xpc_disengage_max_timelimit or fscache_op_wq, but it's very unlikely > that someone writes, for example, 5 into a variable named "zero". If > it does, it most likely a bug, so const is our friend. I completely agree. :) I just wanted to make sure and it turned out the grep was very easy. -- Kees Cook ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] kernel: use sysctl shared variables for range check 2019-04-10 21:50 ` Kees Cook @ 2019-04-10 22:30 ` Matteo Croce 2019-04-10 22:34 ` Kees Cook 0 siblings, 1 reply; 14+ messages in thread From: Matteo Croce @ 2019-04-10 22:30 UTC (permalink / raw) To: Kees Cook Cc: Andrew Morton, LKML, linux-fsdevel, Luis Chamberlain, Alexey Dobriyan On Wed, Apr 10, 2019 at 11:51 PM Kees Cook <keescook@chromium.org> wrote: > > On Wed, Apr 10, 2019 at 12:24 PM Matteo Croce <mcroce@redhat.com> wrote: > > > > On Wed, Apr 10, 2019 at 8:46 PM Kees Cook <keescook@chromium.org> wrote: > > > > > > On Mon, Apr 8, 2019 at 3:09 PM Matteo Croce <mcroce@redhat.com> wrote: > > > > > > > > Use the shared variables for range check, instead of declaring a local one > > > > in every source file. > > > > > > I was expecting this to be a tree-wide change for all the cases found > > > by patch 1's "git grep". > > > > > > > Hi Kees, > > > > I have already the whole patch ready, but I was frightened by the > > output of get_maintainer.pl, so I decided to split the patch into > > small pieces and send the first one. > > Heh, sounds fine. Normally the big tree-wide changes go via Linus just > before cutting rc1 (or rc2). This is "only" 31 source files, though, > so maybe akpm wants to take these instead? Andrew, how do you feel > about that? > FYI, this are the stats from my local repo, just to let you the size of a series with all the changes in it: $ git --no-pager log --stat --oneline linus/master acebb1f752e9 x86: use sysctl shared variables for range check arch/x86/entry/vdso/vdso32-setup.c | 7 ++----- arch/x86/kernel/itmt.c | 6 ++---- 2 files changed, 4 insertions(+), 9 deletions(-) 6731f419f46d s390: use sysctl shared variables for range check arch/s390/appldata/appldata_base.c | 15 +++++---------- arch/s390/kernel/topology.c | 6 ++---- 2 files changed, 7 insertions(+), 14 deletions(-) 1189495100c6 drivers: use sysctl shared variables for range check drivers/base/firmware_loader/fallback_table.c | 11 ++++------- drivers/gpu/drm/i915/i915_perf.c | 8 +++----- drivers/hv/vmbus_drv.c | 6 ++---- drivers/s390/char/sclp_async.c | 7 ++----- drivers/tty/tty_ldisc.c | 6 ++---- drivers/xen/balloon.c | 7 ++----- 6 files changed, 15 insertions(+), 30 deletions(-) bada6ce1f240 ipc: use sysctl shared variables for range check ipc/ipc_sysctl.c | 35 ++++++++++++++++------------------- 1 file changed, 16 insertions(+), 19 deletions(-) 1faa590d64c5 inotify: use sysctl shared variables for range check fs/notify/inotify/inotify_user.c | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) ad4254677a52 security: use sysctl shared variables for range check security/keys/sysctl.c | 26 ++++++++++++-------------- security/loadpin/loadpin.c | 6 ++---- security/yama/yama_lsm.c | 3 +-- 3 files changed, 15 insertions(+), 20 deletions(-) 3d2211ab8c7c net: use sysctl shared variables for range check net/core/neighbour.c | 20 ++++++------- net/core/sysctl_net_core.c | 34 ++++++++++------------ net/dccp/sysctl.c | 16 +++++----- net/ipv4/sysctl_net_ipv4.c | 58 ++++++++++++++++++------------------- net/ipv6/addrconf.c | 6 ++-- net/ipv6/route.c | 7 ++--- net/ipv6/sysctl_net_ipv6.c | 8 ++--- net/mpls/af_mpls.c | 10 +++---- net/netfilter/ipvs/ip_vs_ctl.c | 3 +- net/rxrpc/sysctl.c | 9 +++--- net/sctp/sysctl.c | 35 ++++++++++------------ net/sunrpc/xprtrdma/transport.c | 3 +- 12 files changed, 93 insertions(+), 116 deletions(-) f20fd0e406ec kernel: use sysctl shared variables for range check kernel/pid_namespace.c | 3 +- kernel/sysctl.c | 193 ++++++++++++++++++++++----------------------- kernel/ucount.c | 6 +- 3 files changed, 98 insertions(+), 104 deletions(-) 05fc54289d17 proc/sysctl: add shared variables for range check fs/proc/proc_sysctl.c | 5 +++++ include/linux/sysctl.h | 4 ++++ 2 files changed, 9 insertions(+) -- Matteo Croce per aspera ad upstream ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] kernel: use sysctl shared variables for range check 2019-04-10 22:30 ` Matteo Croce @ 2019-04-10 22:34 ` Kees Cook 2019-04-10 22:54 ` Matteo Croce 0 siblings, 1 reply; 14+ messages in thread From: Kees Cook @ 2019-04-10 22:34 UTC (permalink / raw) To: Matteo Croce Cc: Andrew Morton, LKML, linux-fsdevel, Luis Chamberlain, Alexey Dobriyan On Wed, Apr 10, 2019 at 3:30 PM Matteo Croce <mcroce@redhat.com> wrote: > > On Wed, Apr 10, 2019 at 11:51 PM Kees Cook <keescook@chromium.org> wrote: > > > > On Wed, Apr 10, 2019 at 12:24 PM Matteo Croce <mcroce@redhat.com> wrote: > > > > > > On Wed, Apr 10, 2019 at 8:46 PM Kees Cook <keescook@chromium.org> wrote: > > > > > > > > On Mon, Apr 8, 2019 at 3:09 PM Matteo Croce <mcroce@redhat.com> wrote: > > > > > > > > > > Use the shared variables for range check, instead of declaring a local one > > > > > in every source file. > > > > > > > > I was expecting this to be a tree-wide change for all the cases found > > > > by patch 1's "git grep". > > > > > > > > > > Hi Kees, > > > > > > I have already the whole patch ready, but I was frightened by the > > > output of get_maintainer.pl, so I decided to split the patch into > > > small pieces and send the first one. > > > > Heh, sounds fine. Normally the big tree-wide changes go via Linus just > > before cutting rc1 (or rc2). This is "only" 31 source files, though, > > so maybe akpm wants to take these instead? Andrew, how do you feel > > about that? > > > > FYI, this are the stats from my local repo, just to let you the size > of a series with all the changes in it: > > $ git --no-pager log --stat --oneline linus/master > 2 files changed, 4 insertions(+), 9 deletions(-) > 2 files changed, 7 insertions(+), 14 deletions(-) > 6 files changed, 15 insertions(+), 30 deletions(-) > 1 file changed, 16 insertions(+), 19 deletions(-) > 1 file changed, 3 insertions(+), 5 deletions(-) > 3 files changed, 15 insertions(+), 20 deletions(-) > 12 files changed, 93 insertions(+), 116 deletions(-) > 3 files changed, 98 insertions(+), 104 deletions(-) > 2 files changed, 9 insertions(+) Tiny! :) Seriously, though, I think this should be fine to take directly to Linus after patch 1 lands, or see if akpm wants to carry it directly. -- Kees Cook ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] kernel: use sysctl shared variables for range check 2019-04-10 22:34 ` Kees Cook @ 2019-04-10 22:54 ` Matteo Croce 2019-04-10 22:59 ` Kees Cook 0 siblings, 1 reply; 14+ messages in thread From: Matteo Croce @ 2019-04-10 22:54 UTC (permalink / raw) To: Kees Cook Cc: Andrew Morton, LKML, linux-fsdevel, Luis Chamberlain, Alexey Dobriyan On Thu, Apr 11, 2019 at 12:34 AM Kees Cook <keescook@chromium.org> wrote: > > On Wed, Apr 10, 2019 at 3:30 PM Matteo Croce <mcroce@redhat.com> wrote: > > > > FYI, this are the stats from my local repo, just to let you the size > > of a series with all the changes in it: > > > > $ git --no-pager log --stat --oneline linus/master > > 2 files changed, 4 insertions(+), 9 deletions(-) > > 2 files changed, 7 insertions(+), 14 deletions(-) > > 6 files changed, 15 insertions(+), 30 deletions(-) > > 1 file changed, 16 insertions(+), 19 deletions(-) > > 1 file changed, 3 insertions(+), 5 deletions(-) > > 3 files changed, 15 insertions(+), 20 deletions(-) > > 12 files changed, 93 insertions(+), 116 deletions(-) > > 3 files changed, 98 insertions(+), 104 deletions(-) > > 2 files changed, 9 insertions(+) > > Tiny! :) Seriously, though, I think this should be fine to take > directly to Linus after patch 1 lands, or see if akpm wants to carry > it directly. > > -- > Kees Cook Nice. Still as an 8 patches series, or squashed into a bigger one? -- Matteo Croce per aspera ad upstream ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] kernel: use sysctl shared variables for range check 2019-04-10 22:54 ` Matteo Croce @ 2019-04-10 22:59 ` Kees Cook 2019-04-16 23:45 ` Andrew Morton 0 siblings, 1 reply; 14+ messages in thread From: Kees Cook @ 2019-04-10 22:59 UTC (permalink / raw) To: Matteo Croce Cc: Andrew Morton, LKML, linux-fsdevel, Luis Chamberlain, Alexey Dobriyan On Wed, Apr 10, 2019 at 3:54 PM Matteo Croce <mcroce@redhat.com> wrote: > > On Thu, Apr 11, 2019 at 12:34 AM Kees Cook <keescook@chromium.org> wrote: > > > > On Wed, Apr 10, 2019 at 3:30 PM Matteo Croce <mcroce@redhat.com> wrote: > > > > > > FYI, this are the stats from my local repo, just to let you the size > > > of a series with all the changes in it: > > > > > > $ git --no-pager log --stat --oneline linus/master > > > 2 files changed, 4 insertions(+), 9 deletions(-) > > > 2 files changed, 7 insertions(+), 14 deletions(-) > > > 6 files changed, 15 insertions(+), 30 deletions(-) > > > 1 file changed, 16 insertions(+), 19 deletions(-) > > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > 3 files changed, 15 insertions(+), 20 deletions(-) > > > 12 files changed, 93 insertions(+), 116 deletions(-) > > > 3 files changed, 98 insertions(+), 104 deletions(-) > > > 2 files changed, 9 insertions(+) > > > > Tiny! :) Seriously, though, I think this should be fine to take > > directly to Linus after patch 1 lands, or see if akpm wants to carry > > it directly. > > > > -- > > Kees Cook > > Nice. Still as an 8 patches series, or squashed into a bigger one? I suspect a single patch would be fine, but let's wait to hear from akpm. -- Kees Cook ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] kernel: use sysctl shared variables for range check 2019-04-10 22:59 ` Kees Cook @ 2019-04-16 23:45 ` Andrew Morton 0 siblings, 0 replies; 14+ messages in thread From: Andrew Morton @ 2019-04-16 23:45 UTC (permalink / raw) To: Kees Cook Cc: Matteo Croce, LKML, linux-fsdevel, Luis Chamberlain, Alexey Dobriyan On Wed, 10 Apr 2019 15:59:55 -0700 Kees Cook <keescook@chromium.org> wrote: > On Wed, Apr 10, 2019 at 3:54 PM Matteo Croce <mcroce@redhat.com> wrote: > > > > On Thu, Apr 11, 2019 at 12:34 AM Kees Cook <keescook@chromium.org> wrote: > > > > > > On Wed, Apr 10, 2019 at 3:30 PM Matteo Croce <mcroce@redhat.com> wrote: > > > > > > > > FYI, this are the stats from my local repo, just to let you the size > > > > of a series with all the changes in it: > > > > > > > > $ git --no-pager log --stat --oneline linus/master > > > > 2 files changed, 4 insertions(+), 9 deletions(-) > > > > 2 files changed, 7 insertions(+), 14 deletions(-) > > > > 6 files changed, 15 insertions(+), 30 deletions(-) > > > > 1 file changed, 16 insertions(+), 19 deletions(-) > > > > 1 file changed, 3 insertions(+), 5 deletions(-) > > > > 3 files changed, 15 insertions(+), 20 deletions(-) > > > > 12 files changed, 93 insertions(+), 116 deletions(-) > > > > 3 files changed, 98 insertions(+), 104 deletions(-) > > > > 2 files changed, 9 insertions(+) > > > > > > Tiny! :) Seriously, though, I think this should be fine to take > > > directly to Linus after patch 1 lands, or see if akpm wants to carry > > > it directly. > > > > > > -- > > > Kees Cook > > > > Nice. Still as an 8 patches series, or squashed into a bigger one? > > I suspect a single patch would be fine, but let's wait to hear from akpm. Bring it! ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] kernel: use sysctl shared variables for range check 2019-04-08 22:09 ` [PATCH 2/2] kernel: use sysctl " Matteo Croce 2019-04-10 18:46 ` Kees Cook @ 2019-04-17 3:21 ` Kees Cook 2019-04-17 3:22 ` Kees Cook 1 sibling, 1 reply; 14+ messages in thread From: Kees Cook @ 2019-04-17 3:21 UTC (permalink / raw) To: Matteo Croce Cc: LKML, linux-fsdevel, Luis Chamberlain, Kees Cook, Alexey Dobriyan On Mon, Apr 8, 2019 at 5:09 PM Matteo Croce <mcroce@redhat.com> wrote: > > Use the shared variables for range check, instead of declaring a local one > in every source file. > > Signed-off-by: Matteo Croce <mcroce@redhat.com> > --- > kernel/pid_namespace.c | 3 +- > kernel/sysctl.c | 193 ++++++++++++++++++++--------------------- > kernel/ucount.c | 6 +- > 3 files changed, 98 insertions(+), 104 deletions(-) > > diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c > index aa6e72fb7c08..ddbb51bc4968 100644 > --- a/kernel/pid_namespace.c > +++ b/kernel/pid_namespace.c > @@ -290,14 +290,13 @@ static int pid_ns_ctl_handler(struct ctl_table *table, int write, > } > > extern int pid_max; > -static int zero = 0; > static struct ctl_table pid_ns_ctl_table[] = { > { > .procname = "ns_last_pid", > .maxlen = sizeof(int), > .mode = 0666, /* permissions are checked in the handler */ > .proc_handler = pid_ns_ctl_handler, > - .extra1 = &zero, > + .extra1 = (void *)&sysctl_zero, BTW, I don't think these (void *) casts are actually needed. I thought extra1/2 were already void * so assignments don't need the casting. -- Kees Cook ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 2/2] kernel: use sysctl shared variables for range check 2019-04-17 3:21 ` Kees Cook @ 2019-04-17 3:22 ` Kees Cook 0 siblings, 0 replies; 14+ messages in thread From: Kees Cook @ 2019-04-17 3:22 UTC (permalink / raw) To: Matteo Croce Cc: LKML, linux-fsdevel, Luis Chamberlain, Kees Cook, Alexey Dobriyan On Tue, Apr 16, 2019 at 10:21 PM Kees Cook <keescook@chromium.org> wrote: > > On Mon, Apr 8, 2019 at 5:09 PM Matteo Croce <mcroce@redhat.com> wrote: > > > > Use the shared variables for range check, instead of declaring a local one > > in every source file. > > > > Signed-off-by: Matteo Croce <mcroce@redhat.com> > > --- > > kernel/pid_namespace.c | 3 +- > > kernel/sysctl.c | 193 ++++++++++++++++++++--------------------- > > kernel/ucount.c | 6 +- > > 3 files changed, 98 insertions(+), 104 deletions(-) > > > > diff --git a/kernel/pid_namespace.c b/kernel/pid_namespace.c > > index aa6e72fb7c08..ddbb51bc4968 100644 > > --- a/kernel/pid_namespace.c > > +++ b/kernel/pid_namespace.c > > @@ -290,14 +290,13 @@ static int pid_ns_ctl_handler(struct ctl_table *table, int write, > > } > > > > extern int pid_max; > > -static int zero = 0; > > static struct ctl_table pid_ns_ctl_table[] = { > > { > > .procname = "ns_last_pid", > > .maxlen = sizeof(int), > > .mode = 0666, /* permissions are checked in the handler */ > > .proc_handler = pid_ns_ctl_handler, > > - .extra1 = &zero, > > + .extra1 = (void *)&sysctl_zero, > > BTW, I don't think these (void *) casts are actually needed. I thought > extra1/2 were already void * so assignments don't need the casting. Nevermind, I see akpm already mentioned this, and I see it's the "const" removal now. -- Kees Cook ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2019-04-17 3:22 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-04-08 22:09 [PATCH 0/2] sysctl: share commonly used constants Matteo Croce 2019-04-08 22:09 ` [PATCH 1/2] proc/sysctl: add shared variables for range check Matteo Croce 2019-04-10 22:18 ` Kees Cook 2019-04-08 22:09 ` [PATCH 2/2] kernel: use sysctl " Matteo Croce 2019-04-10 18:46 ` Kees Cook 2019-04-10 19:23 ` Matteo Croce 2019-04-10 21:50 ` Kees Cook 2019-04-10 22:30 ` Matteo Croce 2019-04-10 22:34 ` Kees Cook 2019-04-10 22:54 ` Matteo Croce 2019-04-10 22:59 ` Kees Cook 2019-04-16 23:45 ` Andrew Morton 2019-04-17 3:21 ` Kees Cook 2019-04-17 3:22 ` Kees Cook
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).