Remove CTL_UNNUMBERED
Subsystems (16)
PATCH 1/1 Remove CTL_UNNUMBERED
CTL_UNNUMBERED is unneeded, because it expands to
.ctl_name = 0
The same effect can be achieved by skipping .ctl_name initialization, saving one line per sysctl.
Update docs and headers telling people to not add CTL_ numbers and giving example.
This is probably all we can do to stop the flow of new CTL_ numbers, because most of sysctls are copy-pasted. CTL_UNNUMBERED doesn't solve this problem at all.
---
Documentation/sysctl/ctl_unnumbered.txt | 15 ++++++++++----- arch/ia64/kernel/crash.c | 1 - arch/ia64/kernel/perfmon.c | 5 ----- arch/ia64/sn/kernel/xpc_main.c | 5 ----- arch/mips/au1000/common/power.c | 4 ---- arch/sh64/kernel/traps.c | 5 ----- drivers/char/hpet.c | 2 -- drivers/char/rtc.c | 2 -- fs/coda/sysctl.c | 4 ---- fs/lockd/svc.c | 7 ------- fs/nfs/sysctl.c | 5 ----- fs/ntfs/sysctl.c | 1 - include/linux/sysctl.h | 6 ++---- kernel/sysctl.c | 20 -------------------- net/9p/sysctl.c | 2 -- net/core/sysctl_net_core.c | 2 -- net/ipv6/addrconf.c | 1 - net/netfilter/nf_conntrack_proto_udplite.c | 2 -- net/netfilter/nf_conntrack_standalone.c | 1 - 19 files changed, 12 insertions(+), 78 deletions(-)
Documentation/sysctl/ctl_unnumbered.txt +10 -5
@@ -3,10 +3,6 @@ Except for a few extremely rare exceptions user space applications do not use the binary sysctl interface. Instead everyone uses /proc/sys/... with readable ascii names. -Recently the kernel has started supporting setting the binary sysctl value to -CTL_UNNUMBERED so we no longer need to assign a binary sysctl path to allow -sysctls to show up in /proc/sys. - Assigning binary sysctl numbers is an endless source of conflicts in sysctl.h, breaking of the user space ABI (because of those conflicts), and maintenance problems. A complete pass through all of the sysctl users revealed multiple
@@ -14,7 +10,16 @@ instances where the sysctl binary interface was broken and had gone undetected for years. So please do not add new binary sysctl numbers. They are unneeded and -problematic. +problematic. Instead, use C99 initalizers, skip .ctl_name, and initialize only +.procname: + + { + .procname = "print-fatal-signals", + .data = &print_fatal_signals, + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = &proc_dointvec, + }, If you really need a new binary sysctl number please first merge your sysctl into the kernel and then as a separate patch allocate a binary sysctl number.
arch/ia64/kernel/crash.c +0 -1
@@ -197,7 +197,6 @@ kdump_init_notifier(struct notifier_block *self, unsigned long val, void *data) #ifdef CONFIG_SYSCTL static ctl_table kdump_on_init_table[] = { { - .ctl_name = CTL_UNNUMBERED, .procname = "kdump_on_init", .data = &kdump_on_init, .maxlen = sizeof(int),
arch/ia64/kernel/perfmon.c +0 -5
@@ -521,7 +521,6 @@ EXPORT_SYMBOL(pfm_sysctl);static ctl_table pfm_ctl_table[]={ { - .ctl_name =CTL_UNNUMBERED , .procname ="debug" , .data = &pfm_sysctl.debug, .maxlen =sizeof (int ),
@@ -529,7 +528,6 @@ static ctl_table pfm_ctl_table[]={ .proc_handler = &proc_dointvec, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "debug_ovfl", .data = &pfm_sysctl.debug_ovfl, .maxlen = sizeof(int),
@@ -537,7 +535,6 @@ static ctl_table pfm_ctl_table[]={ .proc_handler = &proc_dointvec, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "fastctxsw", .data = &pfm_sysctl.fastctxsw, .maxlen = sizeof(int),
@@ -545,7 +542,6 @@ static ctl_table pfm_ctl_table[]={ .proc_handler = &proc_dointvec, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "expert_mode", .data = &pfm_sysctl.expert_mode, .maxlen = sizeof(int),
@@ -556,7 +552,6 @@ static ctl_table pfm_ctl_table[]={ };static ctl_table pfm_sysctl_dir[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="perfmon" , .mode =0755 , .child = pfm_ctl_table,
arch/ia64/sn/kernel/xpc_main.c +0 -5
@@ -101,7 +101,6 @@ static int xpc_disengage_request_max_timelimit = 120;static ctl_table xpc_sys_xpc_hb_dir[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="hb_interval" , .data = &xpc_hb_interval, .maxlen =sizeof (int ),
@@ -112,7 +111,6 @@ static ctl_table xpc_sys_xpc_hb_dir[] = { .extra2 = &xpc_hb_max_interval }, { - .ctl_name = CTL_UNNUMBERED, .procname = "hb_check_interval", .data = &xpc_hb_check_interval, .maxlen = sizeof(int),
@@ -126,13 +124,11 @@ static ctl_table xpc_sys_xpc_hb_dir[] = { };static ctl_table xpc_sys_xpc_dir[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="hb" , .mode =0555 , .child = xpc_sys_xpc_hb_dir }, { - .ctl_name =CTL_UNNUMBERED , .procname ="disengage_request_timelimit" , .data = &xpc_disengage_request_timelimit, .maxlen =sizeof (int ),
@@ -146,7 +142,6 @@ static ctl_table xpc_sys_xpc_dir[] = { };static ctl_table xpc_sys_dir[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="xpc" , .mode =0555 , .child = xpc_sys_xpc_dir
arch/mips/au1000/common/power.c +0 -4
@@ -420,7 +420,6 @@ static int pm_do_freq(ctl_table * ctl, int write, struct file *file,static struct ctl_table pm_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="suspend" , .data =NULL , .maxlen =0 ,
@@ -428,7 +427,6 @@ static struct ctl_table pm_table[] = { .proc_handler = &pm_do_suspend }, { - .ctl_name = CTL_UNNUMBERED, .procname = "sleep", .data = NULL, .maxlen = 0,
@@ -436,7 +434,6 @@ static struct ctl_table pm_table[] = { .proc_handler = &pm_do_sleep }, { - .ctl_name = CTL_UNNUMBERED, .procname = "freq", .data = NULL, .maxlen = 0,
@@ -448,7 +445,6 @@ static struct ctl_table pm_table[] = {static struct ctl_table pm_dir_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="pm" , .mode =0555 , .child = pm_table
arch/sh64/kernel/traps.c +0 -5
@@ -910,7 +910,6 @@ static int misaligned_fixup(struct pt_regs *regs)static ctl_table unaligned_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="kernel_reports" , .data = &kernel_mode_unaligned_fixup_count, .maxlen =sizeof (int ),
@@ -919,7 +918,6 @@ static ctl_table unaligned_table[] = { },#if defined(CONFIG_SH64_USER_MISALIGNED_FIXUP ) { - .ctl_name = CTL_UNNUMBERED, .procname = "user_reports", .data = &user_mode_unaligned_fixup_count, .maxlen = sizeof(int),
@@ -927,7 +925,6 @@ static ctl_table unaligned_table[] = { .proc_handler = &proc_dointvec }, { - .ctl_name = CTL_UNNUMBERED, .procname = "user_enable", .data = &user_mode_unaligned_fixup_enable, .maxlen = sizeof(int),
@@ -939,7 +936,6 @@ static ctl_table unaligned_table[] = {static ctl_table unaligned_root[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="unaligned_fixup" , .mode =0555 , unaligned_table
@@ -949,7 +945,6 @@ static ctl_table unaligned_root[] = {static ctl_table sh64_root[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="sh64" , .mode =0555 , .child = unaligned_root
drivers/char/hpet.c +0 -2
@@ -723,7 +723,6 @@ int hpet_control(struct hpet_task *tp, unsigned int cmd, unsigned long arg)static ctl_table hpet_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="max-user-freq" , .data = &hpet_max_freq, .maxlen =sizeof (int ),
@@ -735,7 +734,6 @@ static ctl_table hpet_table[] = {static ctl_table hpet_root[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="hpet" , .maxlen =0 , .mode =0555 ,
drivers/char/rtc.c +0 -2
@@ -279,7 +279,6 @@ irqreturn_t rtc_interrupt(int irq, void *dev_id)*/ static ctl_table rtc_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="max-user-freq" , .data = &rtc_max_user_freq, .maxlen =sizeof (int ),
@@ -291,7 +290,6 @@ static ctl_table rtc_table[] = {static ctl_table rtc_root[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="rtc" , .mode =0555 , .child = rtc_table,
fs/coda/sysctl.c +0 -4
@@ -15,7 +15,6 @@ static struct ctl_table_header *fs_table_header;static ctl_table coda_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="timeout" , .data = &coda_timeout, .maxlen =sizeof (int ),
@@ -23,7 +22,6 @@ static ctl_table coda_table[] = { .proc_handler = &proc_dointvec }, { - .ctl_name = CTL_UNNUMBERED, .procname = "hard", .data = &coda_hard, .maxlen = sizeof(int),
@@ -31,7 +29,6 @@ static ctl_table coda_table[] = { .proc_handler = &proc_dointvec }, { - .ctl_name = CTL_UNNUMBERED, .procname = "fake_statfs", .data = &coda_fake_statfs, .maxlen = sizeof(int),
@@ -43,7 +40,6 @@ static ctl_table coda_table[] = {static ctl_table fs_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="coda" , .mode =0555 , .child = coda_table
fs/lockd/svc.c +0 -7
@@ -370,7 +370,6 @@ EXPORT_SYMBOL(lockd_down);static ctl_table nlm_sysctls[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="nlm_grace_period" , .data = &nlm_grace_period, .maxlen =sizeof (unsigned long ),
@@ -380,7 +379,6 @@ static ctl_table nlm_sysctls[] = { .extra2 = (unsigned long *) &nlm_grace_period_max, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nlm_timeout", .data = &nlm_timeout, .maxlen = sizeof(unsigned long),
@@ -390,7 +388,6 @@ static ctl_table nlm_sysctls[] = { .extra2 = (unsigned long *) &nlm_timeout_max, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nlm_udpport", .data = &nlm_udpport, .maxlen = sizeof(int),
@@ -400,7 +397,6 @@ static ctl_table nlm_sysctls[] = { .extra2 = (int *) &nlm_port_max, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nlm_tcpport", .data = &nlm_tcpport, .maxlen = sizeof(int),
@@ -410,7 +406,6 @@ static ctl_table nlm_sysctls[] = { .extra2 = (int *) &nlm_port_max, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nsm_use_hostnames", .data = &nsm_use_hostnames, .maxlen = sizeof(int),
@@ -418,7 +413,6 @@ static ctl_table nlm_sysctls[] = { .proc_handler = &proc_dointvec, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nsm_local_state", .data = &nsm_local_state, .maxlen = sizeof(int),
@@ -430,7 +424,6 @@ static ctl_table nlm_sysctls[] = {static ctl_table nlm_sysctl_dir[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="nfs" , .mode =0555 , .child = nlm_sysctls,
fs/nfs/sysctl.c +0 -5
@@ -22,7 +22,6 @@ static struct ctl_table_header *nfs_callback_sysctl_table; static ctl_table nfs_cb_sysctls[] = { #ifdef CONFIG_NFS_V4 { - .ctl_name = CTL_UNNUMBERED, .procname = "nfs_callback_tcpport", .data = &nfs_callback_set_tcpport, .maxlen = sizeof(int),
@@ -32,7 +31,6 @@ static ctl_table nfs_cb_sysctls[] = { .extra2 = (int *)&nfs_set_port_max, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "idmap_cache_timeout", .data = &nfs_idmap_cache_timeout, .maxlen = sizeof(int),
@@ -42,7 +40,6 @@ static ctl_table nfs_cb_sysctls[] = { },#endif { - .ctl_name = CTL_UNNUMBERED, .procname = "nfs_mountpoint_timeout", .data = &nfs_mountpoint_expiry_timeout, .maxlen = sizeof(nfs_mountpoint_expiry_timeout),
@@ -51,7 +48,6 @@ static ctl_table nfs_cb_sysctls[] = { .strategy = &sysctl_jiffies, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nfs_congestion_kb", .data = &nfs_congestion_kb, .maxlen = sizeof(nfs_congestion_kb),
@@ -63,7 +59,6 @@ static ctl_table nfs_cb_sysctls[] = {static ctl_table nfs_cb_sysctl_dir[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="nfs" , .mode =0555 , .child = nfs_cb_sysctls,
fs/ntfs/sysctl.c +0 -1
@@ -36,7 +36,6 @@/* Definition of the ntfs sysctl. */ static ctl_table ntfs_sysctls[] = { { - .ctl_name =CTL_UNNUMBERED ,/* Binary and text IDs. */ .procname ="ntfs-debug" , .data = &debug_msgs,/* Data pointer and size. */ .maxlen =sizeof (debug_msgs),
include/linux/sysctl.h +2 -4
@@ -15,8 +15,7 @@ ** The kernel will then return -ENOTDIR to any application using ** the old binary interface. ** - ** For new interfaces unless you really need a binary number - ** please use CTL_UNNUMBERED. + ** Do not initialize .ctl_name for new sysctls. ** **************************************************************** ****************************************************************
@@ -54,7 +53,6 @@ struct __sysctl_args {/* For internal pattern-matching use only: */ #ifdef __KERNEL__#define CTL_NONE 0 -#define CTL_UNNUMBERED CTL_NONE/* sysctl without a binary number */ #endif enum
@@ -1021,7 +1019,7 @@ extern ctl_handler sysctl_ms_jiffies;/* A sysctl table is an array of struct ctl_table: */ struct ctl_table { -int ctl_name;/* Binary ID */ +int ctl_name;/* Binary ID, do not use in new sysctls */ const char *procname;/* Text ID for /proc/sys, or zero */ void *data;int maxlen;
kernel/sysctl.c +0 -20
@@ -222,7 +222,6 @@ static unsigned long max_wakeup_granularity_ns = 1000000000; /* 1 second */ static ctl_table kern_table[] = { #ifdef CONFIG_SCHED_DEBUG { - .ctl_name = CTL_UNNUMBERED, .procname = "sched_granularity_ns", .data = &sysctl_sched_granularity, .maxlen = sizeof(unsigned int),
@@ -233,7 +232,6 @@ static ctl_table kern_table[] = { .extra2 = &max_sched_granularity_ns, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "sched_wakeup_granularity_ns", .data = &sysctl_sched_wakeup_granularity, .maxlen = sizeof(unsigned int),
@@ -244,7 +242,6 @@ static ctl_table kern_table[] = { .extra2 = &max_wakeup_granularity_ns, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "sched_batch_wakeup_granularity_ns", .data = &sysctl_sched_batch_wakeup_granularity, .maxlen = sizeof(unsigned int),
@@ -255,7 +252,6 @@ static ctl_table kern_table[] = { .extra2 = &max_wakeup_granularity_ns, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "sched_stat_granularity_ns", .data = &sysctl_sched_stat_granularity, .maxlen = sizeof(unsigned int),
@@ -266,7 +262,6 @@ static ctl_table kern_table[] = { .extra2 = &max_wakeup_granularity_ns, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "sched_runtime_limit_ns", .data = &sysctl_sched_runtime_limit, .maxlen = sizeof(unsigned int),
@@ -277,7 +272,6 @@ static ctl_table kern_table[] = { .extra2 = &max_sched_granularity_ns, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "sched_child_runs_first", .data = &sysctl_sched_child_runs_first, .maxlen = sizeof(unsigned int),
@@ -286,7 +280,6 @@ static ctl_table kern_table[] = { },#ifdef CONFIG_PROVE_LOCKING { - .ctl_name = CTL_UNNUMBERED, .procname = "prove_locking", .data = &prove_locking, .maxlen = sizeof(int),
@@ -296,7 +289,6 @@ static ctl_table kern_table[] = {#endif #ifdef CONFIG_LOCK_STAT { - .ctl_name = CTL_UNNUMBERED, .procname = "lock_stat", .data = &lock_stat, .maxlen = sizeof(int),
@@ -305,7 +297,6 @@ static ctl_table kern_table[] = { },#endif { - .ctl_name = CTL_UNNUMBERED, .procname = "sched_features", .data = &sysctl_sched_features, .maxlen = sizeof(unsigned int),
@@ -331,7 +322,6 @@ static ctl_table kern_table[] = { },#ifdef CONFIG_AUDITSYSCALL { - .ctl_name = CTL_UNNUMBERED, .procname = "audit_argv_kb", .data = &audit_argv_kb, .maxlen = sizeof(int),
@@ -377,7 +367,6 @@ static ctl_table kern_table[] = { },#endif { - .ctl_name = CTL_UNNUMBERED, .procname = "print-fatal-signals", .data = &print_fatal_signals, .maxlen = sizeof(int),
@@ -661,7 +650,6 @@ static ctl_table kern_table[] = { .proc_handler = &proc_dointvec, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "kstack_depth_to_print", .data = &kstack_depth_to_print, .maxlen = sizeof(int),
@@ -731,7 +719,6 @@ static ctl_table kern_table[] = {#endif #ifdef CONFIG_PROC_FS { - .ctl_name = CTL_UNNUMBERED, .procname = "maps_protect", .data = &maps_protect, .maxlen = sizeof(int),
@@ -740,7 +727,6 @@ static ctl_table kern_table[] = { },#endif { - .ctl_name = CTL_UNNUMBERED, .procname = "poweroff_cmd", .data = &poweroff_cmd, .maxlen = POWEROFF_CMD_PATH_LEN,
@@ -872,7 +858,6 @@ static ctl_table vm_table[] = { .proc_handler = &proc_dointvec, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "hugepages_treat_as_movable", .data = &hugepages_treat_as_movable, .maxlen = sizeof(int),
@@ -1005,7 +990,6 @@ static ctl_table vm_table[] = {#endif #ifdef CONFIG_SMP { - .ctl_name = CTL_UNNUMBERED, .procname = "stat_interval", .data = &sysctl_stat_interval, .maxlen = sizeof(sysctl_stat_interval),
@@ -1016,7 +1000,6 @@ static ctl_table vm_table[] = {#endif #ifdef CONFIG_SECURITY { - .ctl_name = CTL_UNNUMBERED, .procname = "mmap_min_addr", .data = &mmap_min_addr, .maxlen = sizeof(unsigned long),
@@ -1025,7 +1008,6 @@ static ctl_table vm_table[] = { },#ifdef CONFIG_NUMA { - .ctl_name = CTL_UNNUMBERED, .procname = "numa_zonelist_order", .data = &numa_zonelist_order, .maxlen = NUMA_ZONELIST_ORDER_LEN,
@@ -1189,7 +1171,6 @@ static ctl_table fs_table[] = { },#if defined(CONFIG_BINFMT_MISC ) || defined(CONFIG_BINFMT_MISC_MODULE ) { - .ctl_name = CTL_UNNUMBERED, .procname = "binfmt_misc", .mode = 0555, .child = binfmt_misc_table,
@@ -1205,7 +1186,6 @@ static ctl_table fs_table[] = { static ctl_table debug_table[] = { #ifdef CONFIG_X86 { - .ctl_name = CTL_UNNUMBERED, .procname = "exception-trace", .data = &show_unhandled_signals, .maxlen = sizeof(int),
net/9p/sysctl.c +0 -2
@@ -31,7 +31,6 @@ static struct ctl_table p9_table[] = { #ifdef CONFIG_NET_9P_DEBUG { - .ctl_name = CTL_UNNUMBERED, .procname = "debug", .data = &p9_debug_level, .maxlen = sizeof(int),
@@ -44,7 +43,6 @@ static struct ctl_table p9_table[] = {static struct ctl_table p9_net_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="9p" , .maxlen =0 , .mode =0555 ,
net/core/sysctl_net_core.c +0 -2
@@ -121,7 +121,6 @@ ctl_table core_table[] = { .proc_handler = &proc_dointvec }, { - .ctl_name = CTL_UNNUMBERED, .procname = "xfrm_larval_drop", .data = &sysctl_xfrm_larval_drop, .maxlen = sizeof(int),
@@ -129,7 +128,6 @@ ctl_table core_table[] = { .proc_handler = &proc_dointvec }, { - .ctl_name = CTL_UNNUMBERED, .procname = "xfrm_acq_expires", .data = &sysctl_xfrm_acq_expires, .maxlen = sizeof(int),
net/ipv6/addrconf.c +0 -1
@@ -4029,7 +4029,6 @@ static struct addrconf_sysctl_table },#ifdef CONFIG_IPV6_OPTIMISTIC_DAD { - .ctl_name = CTL_UNNUMBERED, .procname = "optimistic_dad", .data = &ipv6_devconf.optimistic_dad, .maxlen = sizeof(int),
net/netfilter/nf_conntrack_proto_udplite.c +0 -2
@@ -169,7 +169,6 @@ static unsigned int udplite_sysctl_table_users;static struct ctl_table_header *udplite_sysctl_header;static struct ctl_table udplite_sysctl_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="nf_conntrack_udplite_timeout" , .data = &nf_ct_udplite_timeout, .maxlen =sizeof (unsigned int ),
@@ -177,7 +176,6 @@ static struct ctl_table udplite_sysctl_table[] = { .proc_handler = &proc_dointvec_jiffies, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nf_conntrack_udplite_timeout_stream", .data = &nf_ct_udplite_timeout_stream, .maxlen = sizeof(unsigned int),
net/netfilter/nf_conntrack_standalone.c +0 -1
@@ -366,7 +366,6 @@ static ctl_table nf_ct_sysctl_table[] = { .extra2 = &log_invalid_proto_max, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nf_conntrack_expect_max", .data = &nf_ct_expect_max, .maxlen = sizeof(int),
Quoted 7 lines · Alexey Dobriyan
CTL_UNNUMBEREDis unneeded, because it expands to.ctl_name = 0The same effect can be achieved by skipping .ctl_name initialization, saving one line per sysctl.
Quoted 8 lines
Update docs and headers telling people to not add
CTL_numbers and giving example.This is probably all we can do to stop the flow of new
CTL_numbers, because most of sysctls are copy-pasted.CTL_UNNUMBEREDdoesn't solve this problem at all.Signed-off-by Alexey Dobriyan <[email protected]>
Nack. Not unless you update the documentation and explanations properly.
The important part is that we stop assigning binary numbers. You are removing part of the description of why we can not assign bianry numbers and how that is important.
CTL_UNNUMBERED may be an irritant to you but as for actually using the code I have look and it is about 6 of 1 half dozen of the other.
Eric
Quoted 12 lines · Eric W. Biederman
Quoted 10 lines · Alexey Dobriyan
CTL_UNNUMBEREDis unneeded, because it expands to.ctl_name = 0The same effect can be achieved by skipping .ctl_name initialization, saving one line per sysctl.
Update docs and headers telling people to not add
CTL_numbers and giving example.
Quoted 8 lines
Quoted 5 lines
This is probably all we can do to stop the flow of new
CTL_numbers, because most of sysctls are copy-pasted.CTL_UNNUMBEREDdoesn't solve this problem at all.Signed-off-by Alexey Dobriyan <[email protected]>Nack. Not unless you update the documentation and explanations properly.
They are left in place:
Assigning binary sysctl numbers is an endless source of conflicts in sysctl.h, breaking of the user space ABI (because of those conflicts), and maintenance problems. A complete pass through all of the sysctl users revealed multiple instances where the sysctl binary interface was broken and had gone undetected for years.
Quoted 3 lines
The important part is that we stop assigning binary numbers. You are removing part of the description of why we can not assign bianry numbers and how that is important.
You want me to rewrite that paragraph actually mentioning CTL_UNNUMBERED?
Quoted 2 lines
CTL_UNNUMBEREDmay be an irritant to you but as for actually using the code I have look and it is about 6 of 1 half dozen of the other.
Sorry, -EPARSE.
PATCH 2/2 sysctl: remove CTL_UNNUMBERED
I found why first version should be rejected, and, no, it is not documentation updates. Here is version 2:
[PATCH 2/2] sysctl: remove CTL_UNNUMBERED
CTL_UNNUMBERED is unneeded, because it expands to
.ctl_name = 0
The same effect can be achieved by skipping .ctl_name initialization, saving one line per sysctl.
Also, remove ->strategy callbacks from CTL_UNNUMBERED sysctls, because they aren't going to be called. And remove CTL_NONE, which nobody uses and is synonym for CTL_UNNUMBERED.
---
Documentation/sysctl/ctl_unnumbered.txt | 5 ++--- arch/ia64/kernel/crash.c | 1 - arch/ia64/kernel/perfmon.c | 5 ----- arch/ia64/sn/kernel/xpc_main.c | 8 -------- arch/mips/au1000/common/power.c | 4 ---- arch/sh64/kernel/traps.c | 5 ----- drivers/char/hpet.c | 2 -- drivers/char/rtc.c | 2 -- fs/coda/sysctl.c | 4 ---- fs/lockd/svc.c | 7 ------- fs/nfs/sysctl.c | 7 ------- fs/ntfs/sysctl.c | 1 - include/linux/sysctl.h | 8 +------- kernel/sysctl.c | 28 ---------------------------- net/9p/sysctl.c | 2 -- net/core/sysctl_net_core.c | 2 -- net/ipv6/addrconf.c | 1 - net/netfilter/nf_conntrack_proto_udplite.c | 2 -- net/netfilter/nf_conntrack_standalone.c | 1 - 19 files changed, 3 insertions(+), 92 deletions(-)
Documentation/sysctl/ctl_unnumbered.txt +2 -3
@@ -3,9 +3,8 @@ Except for a few extremely rare exceptions user space applications do not use the binary sysctl interface. Instead everyone uses /proc/sys/... with readable ascii names. -Recently the kernel has started supporting setting the binary sysctl value to -CTL_UNNUMBERED so we no longer need to assign a binary sysctl path to allow -sysctls to show up in /proc/sys. +Kernel now supports sysctls without binary numbers so we no longer need to +assign them to allow sysctls to show up in /proc/sys. Assigning binary sysctl numbers is an endless source of conflicts in sysctl.h, breaking of the user space ABI (because of those conflicts), and maintenance
arch/ia64/kernel/crash.c +0 -1
@@ -197,7 +197,6 @@ kdump_init_notifier(struct notifier_block *self, unsigned long val, void *data) #ifdef CONFIG_SYSCTL static ctl_table kdump_on_init_table[] = { { - .ctl_name = CTL_UNNUMBERED, .procname = "kdump_on_init", .data = &kdump_on_init, .maxlen = sizeof(int),
arch/ia64/kernel/perfmon.c +0 -5
@@ -521,7 +521,6 @@ EXPORT_SYMBOL(pfm_sysctl);static ctl_table pfm_ctl_table[]={ { - .ctl_name =CTL_UNNUMBERED , .procname ="debug" , .data = &pfm_sysctl.debug, .maxlen =sizeof (int ),
@@ -529,7 +528,6 @@ static ctl_table pfm_ctl_table[]={ .proc_handler = &proc_dointvec, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "debug_ovfl", .data = &pfm_sysctl.debug_ovfl, .maxlen = sizeof(int),
@@ -537,7 +535,6 @@ static ctl_table pfm_ctl_table[]={ .proc_handler = &proc_dointvec, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "fastctxsw", .data = &pfm_sysctl.fastctxsw, .maxlen = sizeof(int),
@@ -545,7 +542,6 @@ static ctl_table pfm_ctl_table[]={ .proc_handler = &proc_dointvec, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "expert_mode", .data = &pfm_sysctl.expert_mode, .maxlen = sizeof(int),
@@ -556,7 +552,6 @@ static ctl_table pfm_ctl_table[]={ };static ctl_table pfm_sysctl_dir[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="perfmon" , .mode =0755 , .child = pfm_ctl_table,
arch/ia64/sn/kernel/xpc_main.c +0 -8
@@ -101,24 +101,20 @@ static int xpc_disengage_request_max_timelimit = 120;static ctl_table xpc_sys_xpc_hb_dir[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="hb_interval" , .data = &xpc_hb_interval, .maxlen =sizeof (int ), .mode =0644 , .proc_handler = &proc_dointvec_minmax, - .strategy = &sysctl_intvec, .extra1 = &xpc_hb_min_interval, .extra2 = &xpc_hb_max_interval }, { - .ctl_name =CTL_UNNUMBERED , .procname ="hb_check_interval" , .data = &xpc_hb_check_interval, .maxlen =sizeof (int ), .mode =0644 , .proc_handler = &proc_dointvec_minmax, - .strategy = &sysctl_intvec, .extra1 = &xpc_hb_check_min_interval, .extra2 = &xpc_hb_check_max_interval },
@@ -126,19 +122,16 @@ static ctl_table xpc_sys_xpc_hb_dir[] = { };static ctl_table xpc_sys_xpc_dir[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="hb" , .mode =0555 , .child = xpc_sys_xpc_hb_dir }, { - .ctl_name =CTL_UNNUMBERED , .procname ="disengage_request_timelimit" , .data = &xpc_disengage_request_timelimit, .maxlen =sizeof (int ), .mode =0644 , .proc_handler = &proc_dointvec_minmax, - .strategy = &sysctl_intvec, .extra1 = &xpc_disengage_request_min_timelimit, .extra2 = &xpc_disengage_request_max_timelimit },
@@ -146,7 +139,6 @@ static ctl_table xpc_sys_xpc_dir[] = { };static ctl_table xpc_sys_dir[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="xpc" , .mode =0555 , .child = xpc_sys_xpc_dir
arch/mips/au1000/common/power.c +0 -4
@@ -420,7 +420,6 @@ static int pm_do_freq(ctl_table * ctl, int write, struct file *file,static struct ctl_table pm_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="suspend" , .data =NULL , .maxlen =0 ,
@@ -428,7 +427,6 @@ static struct ctl_table pm_table[] = { .proc_handler = &pm_do_suspend }, { - .ctl_name = CTL_UNNUMBERED, .procname = "sleep", .data = NULL, .maxlen = 0,
@@ -436,7 +434,6 @@ static struct ctl_table pm_table[] = { .proc_handler = &pm_do_sleep }, { - .ctl_name = CTL_UNNUMBERED, .procname = "freq", .data = NULL, .maxlen = 0,
@@ -448,7 +445,6 @@ static struct ctl_table pm_table[] = {static struct ctl_table pm_dir_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="pm" , .mode =0555 , .child = pm_table
arch/sh64/kernel/traps.c +0 -5
@@ -910,7 +910,6 @@ static int misaligned_fixup(struct pt_regs *regs)static ctl_table unaligned_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="kernel_reports" , .data = &kernel_mode_unaligned_fixup_count, .maxlen =sizeof (int ),
@@ -919,7 +918,6 @@ static ctl_table unaligned_table[] = { },#if defined(CONFIG_SH64_USER_MISALIGNED_FIXUP ) { - .ctl_name = CTL_UNNUMBERED, .procname = "user_reports", .data = &user_mode_unaligned_fixup_count, .maxlen = sizeof(int),
@@ -927,7 +925,6 @@ static ctl_table unaligned_table[] = { .proc_handler = &proc_dointvec }, { - .ctl_name = CTL_UNNUMBERED, .procname = "user_enable", .data = &user_mode_unaligned_fixup_enable, .maxlen = sizeof(int),
@@ -939,7 +936,6 @@ static ctl_table unaligned_table[] = {static ctl_table unaligned_root[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="unaligned_fixup" , .mode =0555 , unaligned_table
@@ -949,7 +945,6 @@ static ctl_table unaligned_root[] = {static ctl_table sh64_root[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="sh64" , .mode =0555 , .child = unaligned_root
drivers/char/hpet.c +0 -2
@@ -723,7 +723,6 @@ int hpet_control(struct hpet_task *tp, unsigned int cmd, unsigned long arg)static ctl_table hpet_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="max-user-freq" , .data = &hpet_max_freq, .maxlen =sizeof (int ),
@@ -735,7 +734,6 @@ static ctl_table hpet_table[] = {static ctl_table hpet_root[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="hpet" , .maxlen =0 , .mode =0555 ,
drivers/char/rtc.c +0 -2
@@ -279,7 +279,6 @@ irqreturn_t rtc_interrupt(int irq, void *dev_id)*/ static ctl_table rtc_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="max-user-freq" , .data = &rtc_max_user_freq, .maxlen =sizeof (int ),
@@ -291,7 +290,6 @@ static ctl_table rtc_table[] = {static ctl_table rtc_root[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="rtc" , .mode =0555 , .child = rtc_table,
fs/coda/sysctl.c +0 -4
@@ -15,7 +15,6 @@ static struct ctl_table_header *fs_table_header;static ctl_table coda_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="timeout" , .data = &coda_timeout, .maxlen =sizeof (int ),
@@ -23,7 +22,6 @@ static ctl_table coda_table[] = { .proc_handler = &proc_dointvec }, { - .ctl_name = CTL_UNNUMBERED, .procname = "hard", .data = &coda_hard, .maxlen = sizeof(int),
@@ -31,7 +29,6 @@ static ctl_table coda_table[] = { .proc_handler = &proc_dointvec }, { - .ctl_name = CTL_UNNUMBERED, .procname = "fake_statfs", .data = &coda_fake_statfs, .maxlen = sizeof(int),
@@ -43,7 +40,6 @@ static ctl_table coda_table[] = {static ctl_table fs_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="coda" , .mode =0555 , .child = coda_table
fs/lockd/svc.c +0 -7
@@ -370,7 +370,6 @@ EXPORT_SYMBOL(lockd_down);static ctl_table nlm_sysctls[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="nlm_grace_period" , .data = &nlm_grace_period, .maxlen =sizeof (unsigned long ),
@@ -380,7 +379,6 @@ static ctl_table nlm_sysctls[] = { .extra2 = (unsigned long *) &nlm_grace_period_max, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nlm_timeout", .data = &nlm_timeout, .maxlen = sizeof(unsigned long),
@@ -390,7 +388,6 @@ static ctl_table nlm_sysctls[] = { .extra2 = (unsigned long *) &nlm_timeout_max, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nlm_udpport", .data = &nlm_udpport, .maxlen = sizeof(int),
@@ -400,7 +397,6 @@ static ctl_table nlm_sysctls[] = { .extra2 = (int *) &nlm_port_max, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nlm_tcpport", .data = &nlm_tcpport, .maxlen = sizeof(int),
@@ -410,7 +406,6 @@ static ctl_table nlm_sysctls[] = { .extra2 = (int *) &nlm_port_max, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nsm_use_hostnames", .data = &nsm_use_hostnames, .maxlen = sizeof(int),
@@ -418,7 +413,6 @@ static ctl_table nlm_sysctls[] = { .proc_handler = &proc_dointvec, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nsm_local_state", .data = &nsm_local_state, .maxlen = sizeof(int),
@@ -430,7 +424,6 @@ static ctl_table nlm_sysctls[] = {static ctl_table nlm_sysctl_dir[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="nfs" , .mode =0555 , .child = nlm_sysctls,
fs/nfs/sysctl.c +0 -7
@@ -22,7 +22,6 @@ static struct ctl_table_header *nfs_callback_sysctl_table; static ctl_table nfs_cb_sysctls[] = { #ifdef CONFIG_NFS_V4 { - .ctl_name = CTL_UNNUMBERED, .procname = "nfs_callback_tcpport", .data = &nfs_callback_set_tcpport, .maxlen = sizeof(int),
@@ -32,26 +31,21 @@ static ctl_table nfs_cb_sysctls[] = { .extra2 = (int *)&nfs_set_port_max, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "idmap_cache_timeout", .data = &nfs_idmap_cache_timeout, .maxlen = sizeof(int), .mode = 0644, .proc_handler = &proc_dointvec_jiffies, - .strategy = &sysctl_jiffies, },#endif { - .ctl_name = CTL_UNNUMBERED, .procname = "nfs_mountpoint_timeout", .data = &nfs_mountpoint_expiry_timeout, .maxlen = sizeof(nfs_mountpoint_expiry_timeout), .mode = 0644, .proc_handler = &proc_dointvec_jiffies, - .strategy = &sysctl_jiffies, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nfs_congestion_kb", .data = &nfs_congestion_kb, .maxlen = sizeof(nfs_congestion_kb),
@@ -63,7 +57,6 @@ static ctl_table nfs_cb_sysctls[] = {static ctl_table nfs_cb_sysctl_dir[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="nfs" , .mode =0555 , .child = nfs_cb_sysctls,
fs/ntfs/sysctl.c +0 -1
@@ -36,7 +36,6 @@/* Definition of the ntfs sysctl. */ static ctl_table ntfs_sysctls[] = { { - .ctl_name =CTL_UNNUMBERED ,/* Binary and text IDs. */ .procname ="ntfs-debug" , .data = &debug_msgs,/* Data pointer and size. */ .maxlen =sizeof (debug_msgs),
include/linux/sysctl.h +1 -7
@@ -16,7 +16,7 @@ ** the old binary interface. ** ** For new interfaces unless you really need a binary number - ** please use CTL_UNNUMBERED. + ** please do not initialize .ctl_name and .strategy . ** **************************************************************** ****************************************************************
@@ -51,12 +51,6 @@ struct __sysctl_args {/* Top-level names: */ -/* For internal pattern-matching use only: */ -#ifdef __KERNEL__ -#define CTL_NONE 0 -#define CTL_UNNUMBERED CTL_NONE/* sysctl without a binary number */ -#endif -enum {CTL_KERN =1 ,/* General kernel info and control */
kernel/sysctl.c +0 -28
@@ -222,62 +222,51 @@ static unsigned long max_wakeup_granularity_ns = 1000000000; /* 1 second */ static ctl_table kern_table[] = { #ifdef CONFIG_SCHED_DEBUG { - .ctl_name = CTL_UNNUMBERED, .procname = "sched_granularity_ns", .data = &sysctl_sched_granularity, .maxlen = sizeof(unsigned int), .mode = 0644, .proc_handler = &proc_dointvec_minmax, - .strategy = &sysctl_intvec, .extra1 = &min_sched_granularity_ns, .extra2 = &max_sched_granularity_ns, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "sched_wakeup_granularity_ns", .data = &sysctl_sched_wakeup_granularity, .maxlen = sizeof(unsigned int), .mode = 0644, .proc_handler = &proc_dointvec_minmax, - .strategy = &sysctl_intvec, .extra1 = &min_wakeup_granularity_ns, .extra2 = &max_wakeup_granularity_ns, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "sched_batch_wakeup_granularity_ns", .data = &sysctl_sched_batch_wakeup_granularity, .maxlen = sizeof(unsigned int), .mode = 0644, .proc_handler = &proc_dointvec_minmax, - .strategy = &sysctl_intvec, .extra1 = &min_wakeup_granularity_ns, .extra2 = &max_wakeup_granularity_ns, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "sched_stat_granularity_ns", .data = &sysctl_sched_stat_granularity, .maxlen = sizeof(unsigned int), .mode = 0644, .proc_handler = &proc_dointvec_minmax, - .strategy = &sysctl_intvec, .extra1 = &min_wakeup_granularity_ns, .extra2 = &max_wakeup_granularity_ns, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "sched_runtime_limit_ns", .data = &sysctl_sched_runtime_limit, .maxlen = sizeof(unsigned int), .mode = 0644, .proc_handler = &proc_dointvec_minmax, - .strategy = &sysctl_intvec, .extra1 = &min_sched_granularity_ns, .extra2 = &max_sched_granularity_ns, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "sched_child_runs_first", .data = &sysctl_sched_child_runs_first, .maxlen = sizeof(unsigned int),
@@ -286,7 +275,6 @@ static ctl_table kern_table[] = { },#ifdef CONFIG_PROVE_LOCKING { - .ctl_name = CTL_UNNUMBERED, .procname = "prove_locking", .data = &prove_locking, .maxlen = sizeof(int),
@@ -296,7 +284,6 @@ static ctl_table kern_table[] = {#endif #ifdef CONFIG_LOCK_STAT { - .ctl_name = CTL_UNNUMBERED, .procname = "lock_stat", .data = &lock_stat, .maxlen = sizeof(int),
@@ -305,7 +292,6 @@ static ctl_table kern_table[] = { },#endif { - .ctl_name = CTL_UNNUMBERED, .procname = "sched_features", .data = &sysctl_sched_features, .maxlen = sizeof(unsigned int),
@@ -331,7 +317,6 @@ static ctl_table kern_table[] = { },#ifdef CONFIG_AUDITSYSCALL { - .ctl_name = CTL_UNNUMBERED, .procname = "audit_argv_kb", .data = &audit_argv_kb, .maxlen = sizeof(int),
@@ -377,7 +362,6 @@ static ctl_table kern_table[] = { },#endif { - .ctl_name = CTL_UNNUMBERED, .procname = "print-fatal-signals", .data = &print_fatal_signals, .maxlen = sizeof(int),
@@ -661,7 +645,6 @@ static ctl_table kern_table[] = { .proc_handler = &proc_dointvec, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "kstack_depth_to_print", .data = &kstack_depth_to_print, .maxlen = sizeof(int),
@@ -731,7 +714,6 @@ static ctl_table kern_table[] = {#endif #ifdef CONFIG_PROC_FS { - .ctl_name = CTL_UNNUMBERED, .procname = "maps_protect", .data = &maps_protect, .maxlen = sizeof(int),
@@ -740,13 +722,11 @@ static ctl_table kern_table[] = { },#endif { - .ctl_name = CTL_UNNUMBERED, .procname = "poweroff_cmd", .data = &poweroff_cmd, .maxlen = POWEROFF_CMD_PATH_LEN, .mode = 0644, .proc_handler = &proc_dostring, - .strategy = &sysctl_string, },/* * NOTE: do not add new entries to this table unless you have read
@@ -872,7 +852,6 @@ static ctl_table vm_table[] = { .proc_handler = &proc_dointvec, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "hugepages_treat_as_movable", .data = &hugepages_treat_as_movable, .maxlen = sizeof(int),
@@ -1005,18 +984,15 @@ static ctl_table vm_table[] = {#endif #ifdef CONFIG_SMP { - .ctl_name = CTL_UNNUMBERED, .procname = "stat_interval", .data = &sysctl_stat_interval, .maxlen = sizeof(sysctl_stat_interval), .mode = 0644, .proc_handler = &proc_dointvec_jiffies, - .strategy = &sysctl_jiffies, }, #endif #ifdef CONFIG_SECURITY { - .ctl_name = CTL_UNNUMBERED, .procname = "mmap_min_addr", .data = &mmap_min_addr, .maxlen = sizeof(unsigned long),
@@ -1025,13 +1001,11 @@ static ctl_table vm_table[] = { },#ifdef CONFIG_NUMA { - .ctl_name = CTL_UNNUMBERED, .procname = "numa_zonelist_order", .data = &numa_zonelist_order, .maxlen = NUMA_ZONELIST_ORDER_LEN, .mode = 0644, .proc_handler = &numa_zonelist_order_handler, - .strategy = &sysctl_string, },#endif #endif
@@ -1189,7 +1163,6 @@ static ctl_table fs_table[] = { },#if defined(CONFIG_BINFMT_MISC ) || defined(CONFIG_BINFMT_MISC_MODULE ) { - .ctl_name = CTL_UNNUMBERED, .procname = "binfmt_misc", .mode = 0555, .child = binfmt_misc_table,
@@ -1205,7 +1178,6 @@ static ctl_table fs_table[] = { static ctl_table debug_table[] = { #ifdef CONFIG_X86 { - .ctl_name = CTL_UNNUMBERED, .procname = "exception-trace", .data = &show_unhandled_signals, .maxlen = sizeof(int),
net/9p/sysctl.c +0 -2
@@ -31,7 +31,6 @@ static struct ctl_table p9_table[] = { #ifdef CONFIG_NET_9P_DEBUG { - .ctl_name = CTL_UNNUMBERED, .procname = "debug", .data = &p9_debug_level, .maxlen = sizeof(int),
@@ -44,7 +43,6 @@ static struct ctl_table p9_table[] = {static struct ctl_table p9_net_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="9p" , .maxlen =0 , .mode =0555 ,
net/core/sysctl_net_core.c +0 -2
@@ -121,7 +121,6 @@ ctl_table core_table[] = { .proc_handler = &proc_dointvec }, { - .ctl_name = CTL_UNNUMBERED, .procname = "xfrm_larval_drop", .data = &sysctl_xfrm_larval_drop, .maxlen = sizeof(int),
@@ -129,7 +128,6 @@ ctl_table core_table[] = { .proc_handler = &proc_dointvec }, { - .ctl_name = CTL_UNNUMBERED, .procname = "xfrm_acq_expires", .data = &sysctl_xfrm_acq_expires, .maxlen = sizeof(int),
net/ipv6/addrconf.c +0 -1
@@ -4031,7 +4031,6 @@ static struct addrconf_sysctl_table },#ifdef CONFIG_IPV6_OPTIMISTIC_DAD { - .ctl_name = CTL_UNNUMBERED, .procname = "optimistic_dad", .data = &ipv6_devconf.optimistic_dad, .maxlen = sizeof(int),
net/netfilter/nf_conntrack_proto_udplite.c +0 -2
@@ -169,7 +169,6 @@ static unsigned int udplite_sysctl_table_users;static struct ctl_table_header *udplite_sysctl_header;static struct ctl_table udplite_sysctl_table[] = { { - .ctl_name =CTL_UNNUMBERED , .procname ="nf_conntrack_udplite_timeout" , .data = &nf_ct_udplite_timeout, .maxlen =sizeof (unsigned int ),
@@ -177,7 +176,6 @@ static struct ctl_table udplite_sysctl_table[] = { .proc_handler = &proc_dointvec_jiffies, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nf_conntrack_udplite_timeout_stream", .data = &nf_ct_udplite_timeout_stream, .maxlen = sizeof(unsigned int),
net/netfilter/nf_conntrack_standalone.c +0 -1
@@ -366,7 +366,6 @@ static ctl_table nf_ct_sysctl_table[] = { .extra2 = &log_invalid_proto_max, }, { - .ctl_name = CTL_UNNUMBERED, .procname = "nf_conntrack_expect_max", .data = &nf_ct_expect_max, .maxlen = sizeof(int),
Re: [PATCH 2/2] sysctl: remove CTL_UNNUMBERED
Quoted 2 lines · Alexey Dobriyan
I found why first version should be rejected, and, no, it is not documentation updates. Here is version 2:
I care about the documentation because it is the best thing we have for enforcing sane sysctl table usage.
After looking at the problem. While I can't remove ctl_name from the sysctl tables easily. What I can do is check at registration time if a sysctl table is sane, and print
an error message and fail to register that table if it has problems.
Once that is in place I will have no problems killing CTL_UNNUMBERED.
Eric
PATCH 1/3 sysctl core: Stop using the unnecessary ctl_table typedef
In reply to Alexey DobriyanIn sysctl.h the typedef struct ctl_table ctl_table violates coding style isn't needed and is a bit of a nuisance because it makes it harder to recognize ctl_table is a type name.
So this patch removes it from the generic sysctl code. Hopefully I will have enough energy to send the rest of my patches will follow and to remove it from the rest of the kernel.
--- include/linux/sysctl.h | 36 ++++++++-------- kernel/sysctl.c | 114 ++++++++++++++++++++++++------------------------ 2 files changed, 75 insertions(+), 75 deletions(-)
include/linux/sysctl.h +18 -18
@@ -937,37 +937,37 @@ extern int sysctl_perm(struct ctl_table *table, int op);typedef struct ctl_table ctl_table ; -typedef int ctl_handler (ctl_table *table,int __user *name,int nlen, +typedef int ctl_handler (struct ctl_table *table,int __user *name,int nlen,void __user *oldval,size_t __user *oldlenp,void __user *newval,size_t newlen); -typedef int proc_handler (ctl_table *ctl,int write,struct file * filp, +typedef int proc_handler (struct ctl_table *ctl,int write,struct file * filp,void __user *buffer,size_t *lenp,loff_t *ppos); -extern int proc_dostring (ctl_table *,int ,struct file *, +extern int proc_dostring (struct ctl_table *,int ,struct file *,void __user *,size_t *,loff_t *); -extern int proc_dointvec (ctl_table *,int ,struct file *, +extern int proc_dointvec (struct ctl_table *,int ,struct file *,void __user *,size_t *,loff_t *); -extern int proc_dointvec_bset (ctl_table *,int ,struct file *, +extern int proc_dointvec_bset (struct ctl_table *,int ,struct file *,void __user *,size_t *,loff_t *); -extern int proc_dointvec_minmax (ctl_table *,int ,struct file *, +extern int proc_dointvec_minmax (struct ctl_table *,int ,struct file *,void __user *,size_t *,loff_t *); -extern int proc_dointvec_jiffies (ctl_table *,int ,struct file *, +extern int proc_dointvec_jiffies (struct ctl_table *,int ,struct file *,void __user *,size_t *,loff_t *); -extern int proc_dointvec_userhz_jiffies (ctl_table *,int ,struct file *, +extern int proc_dointvec_userhz_jiffies (struct ctl_table *,int ,struct file *,void __user *,size_t *,loff_t *); -extern int proc_dointvec_ms_jiffies (ctl_table *,int ,struct file *, +extern int proc_dointvec_ms_jiffies (struct ctl_table *,int ,struct file *,void __user *,size_t *,loff_t *); -extern int proc_doulongvec_minmax (ctl_table *,int ,struct file *, +extern int proc_doulongvec_minmax (struct ctl_table *,int ,struct file *,void __user *,size_t *,loff_t *); -extern int proc_doulongvec_ms_jiffies_minmax (ctl_table *table,int , +extern int proc_doulongvec_ms_jiffies_minmax (struct ctl_table *table,int ,struct file *,void __user *,size_t *,loff_t *);extern int do_sysctl (int __user *name,int nlen,void __user *oldval,size_t __user *oldlenp,void __user *newval,size_t newlen); -extern int do_sysctl_strategy (ctl_table *table, +extern int do_sysctl_strategy (struct ctl_table *table,int __user *name,int nlen,void __user *oldval,size_t __user *oldlenp,void __user *newval,size_t newlen);
@@ -980,7 +980,7 @@ extern ctl_handler sysctl_ms_jiffies;/* * Register a set of sysctl names by calling register_sysctl_table -* with an initialised array of ctl_table's. An entry with zero +* with an initialised array of struct ctl_table's. An entry with zero * ctl_name and NULL procname terminates the table. table->de will be * set up by the registration and need not be initialised in advance. *
@@ -1026,8 +1026,8 @@ struct ctl_tablevoid *data;int maxlen;mode_t mode; -ctl_table *child; -ctl_table *parent;/* Automatically set */ +struct ctl_table *child; +struct ctl_table *parent;/* Automatically set */ proc_handler *proc_handler;/* Callback for text formatting */ ctl_handler *strategy;/* Callback function for all r/w */ void *extra1;
@@ -1035,16 +1035,16 @@ struct ctl_table };/* struct ctl_table_header is used to maintain dynamic lists of -ctl_table trees. */ +struct ctl_table trees. */ struct ctl_table_header { -ctl_table *ctl_table; +struct ctl_table *ctl_table;struct list_head ctl_entry;int used;struct completion *unregistering; }; -struct ctl_table_header *register_sysctl_table (ctl_table * table); +struct ctl_table_header *register_sysctl_table (struct ctl_table * table);void unregister_sysctl_table (struct ctl_table_header * table);
kernel/sysctl.c +57 -57
394 lines not on this page · raw message
Signature · 1 line
1.5.1.1.181.g2de0
PATCH 2/3 sysctl: Factor out sysctl_data.
There as been no easy way to wrap the default sysctl strategy routine except for returning 0. Which is not always what we want. The few instances I have seen that want different behaviour have written their own version of sysctl_data. While not too hard it is unnecessary code and has the potential for extra bugs.
So to make these situnations easier and make that part of sysctl more symetric I have factord sysctl_data out of do_sysctl_strategy and exported as a function everyone can use.
Further having sysctl_data be an explicit function makes checking for badly formed sysctl tables much easier.
--- include/linux/sysctl.h | 1 + kernel/sysctl.c | 66 +++++++++++++++++++++++++++++++++-------------- 2 files changed, 47 insertions(+), 20 deletions(-)
include/linux/sysctl.h +1 -0
@@ -972,6 +972,7 @@ extern int do_sysctl_strategy (struct ctl_table *table, void __user *oldval, size_t __user *oldlenp, void __user *newval, size_t newlen); +extern ctl_handler sysctl_data;extern ctl_handler sysctl_string;extern ctl_handler sysctl_intvec;extern ctl_handler sysctl_jiffies;
kernel/sysctl.c +46 -20
94 lines not on this page · raw message
Signature · 1 line
1.5.1.1.181.g2de0
PATCH 3/3 sysctl: Error on bad sysctl tables
After going through the kernels sysctl tables several times it has become clear that code review and testing is just not effective in prevent problematic sysctl tables from being used in the stable kernel. I certainly can't seem to fix the problems as fast as they are introduced.
Therefore this patch adds sysctl_check_table which is called when a sysctl table is registered and checks to see if we have a problematic sysctl table.
The biggest part of the code is the table of valid binary sysctl entries, but since we have frozen our set of binary sysctls this table should not need to change, and it makes it much easier to detect when someone unintentionally adds a new binary sysctl value.
As best as I can determine all of the several hundred errors spewed on boot up now are legitimate.
--- include/linux/sysctl.h | 1 + kernel/Makefile | 3 +- kernel/sysctl.c | 6 + kernel/sysctl_check.c | 1555 ++++++++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 1564 insertions(+), 1 deletions(-) create mode 100644 kernel/sysctl_check.c
include/linux/sysctl.h +1 -0
@@ -1048,6 +1048,7 @@ struct ctl_table_headerstruct ctl_table_header *register_sysctl_table (struct ctl_table * table);void unregister_sysctl_table (struct ctl_table_header * table); +int sysctl_check_table (struct ctl_table *table);#else /* __KERNEL__ */
kernel/Makefile +2 -1
@@ -9,8 +9,9 @@ obj-y = sched.o fork.o exec_domain.o panic.o printk.o profile.o \ rcupdate.o extable.o params.o posix-timers.o \ kthread.o wait.o kfifo.o sys_ni.o posix-cpu-timers.o mutex.o \ hrtimer.o rwsem.o latency.o nsproxy.o srcu.o die_notifier.o \ - utsname.o + utsname.o sysctl.o +obj-$(CONFIG_SYSCTL) += sysctl_check.o obj-$(CONFIG_STACKTRACE) += stacktrace.o obj-y += time/ obj-$(CONFIG_DEBUG_MUTEXES) += mutex-debug.o
kernel/sysctl.c +6 -0
@@ -1446,7 +1446,9 @@ static void sysctl_set_parent(struct ctl_table *parent, struct ctl_table *table)static __init int sysctl_init (void ) { +int err;sysctl_set_parent (NULL , root_table); + err =sysctl_check_table (root_table);return 0 ; }
@@ -1531,6 +1533,10 @@ struct ctl_table_header *register_sysctl_table(struct ctl_table * table) tmp->used =0 ; tmp->unregistering =NULL ;sysctl_set_parent (NULL , table); +if (sysctl_check_table (tmp->ctl_table)) { +kfree (tmp); +return NULL ; + }spin_lock (&sysctl_lock);list_add_tail (&tmp->ctl_entry, &root_table_header.ctl_entry);spin_unlock (&sysctl_lock);
kernel/sysctl_check.c new file +1555 -0
1,555 lines not on this page · raw message
Signature · 1 line
1.5.1.1.181.g2de0
PATCH 1/10 sysctl: Update sysctl_check_table
Well it turns out after I dug into the problems a
little more I was returning a few false positives
so this patch updates my logic to remove them.
- Don't complain about 0 ctl_names in sysctl_check_binary_path It is valid for someone to remove the sysctl binary interface and still keep the same sysctl proc interface.
- Count ctl_names and procnames as matching if they both don't exist.
- Only warn about missing min&max when the generic functions care.
--- kernel/sysctl_check.c | 30 ++++++++++++++++-------------- 1 files changed, 16 insertions(+), 14 deletions(-)
kernel/sysctl_check.c +16 -14
@@ -1420,12 +1420,14 @@ static int sysctl_check_dir(struct ctl_table *table) ref =sysctl_check_lookup (table);if (ref) {int match =0 ; -if (table->procname && ref->procname && - (strcmp (table->procname, ref->procname) ==0 )) +if ((!table->procname && !ref->procname) || + (table->procname && ref->procname && + (strcmp (table->procname, ref->procname) ==0 ))) match++; -if (table->ctl_name && ref->ctl_name && - (table->ctl_name == ref->ctl_name)) +if ((!table->ctl_name && !ref->ctl_name) || + (table->ctl_name && ref->ctl_name && + (table->ctl_name == ref->ctl_name))) match++;if (match !=2 ) {
@@ -1462,8 +1464,8 @@ static void sysctl_check_bin_path(struct ctl_table *table, const char **fail) (strcmp(table->procname, ref->procname) != 0)))set_fail (fail, table,"procname does not match binary path procname" ); -if (ref->ctl_name && - (!table->ctl_name || table->ctl_name != ref->ctl_name)) +if (ref->ctl_name && table->ctl_name && + (table->ctl_name != ref->ctl_name))set_fail (fail, table,"ctl_name does not match binary path ctl_name" ); } }
@@ -1499,7 +1501,7 @@ int sysctl_check_table(struct ctl_table *table)if (table->extra2)set_fail (&fail, table,"Directory with extra2" );if (sysctl_check_dir (table)) -set_fail (&fail, table,"Inconsistent directory" ); +set_fail (&fail, table,"Inconsistent directory names" ); }else { if ((table->strategy == sysctl_data) || (table->strategy == sysctl_string) ||
@@ -1520,14 +1522,14 @@ int sysctl_check_table(struct ctl_table *table)if (!table->maxlen)set_fail (&fail, table,"No maxlen" ); } -if ((table->strategy == sysctl_intvec) || - (table->proc_handler == proc_dointvec_minmax) || - (table->proc_handler == proc_doulongvec_minmax) || +if ((table->proc_handler == proc_doulongvec_minmax) || (table->proc_handler == proc_doulongvec_ms_jiffies_minmax)) { -if (!table->extra1) -set_fail (&fail, table,"No min" ); -if (!table->extra2) -set_fail (&fail, table,"No max" ); +if (table->maxlen >sizeof (unsigned long )) { +if (!table->extra1) +set_fail (&fail, table,"No min" ); +if (!table->extra2) +set_fail (&fail, table,"No max" ); + } }if (table->ctl_name && !table->strategy)set_fail (&fail, table,"Missing strategy" );
Signature · 1 line
1.5.1.1.181.g2de0
PATCH 2/10 sysct mqueue: Remove the binary sysctl numbers
Because of a conflict with FS_INODE_NR none of the binary sysctl numbers use by mqueue, were available to user space. So just remove them.
--- ipc/mqueue.c | 10 ---------- 1 files changed, 0 insertions(+), 10 deletions(-)
ipc/mqueue.c +0 -10
@@ -44,12 +44,6 @@#define STATE_PENDING 1#define STATE_READY 2 -/* used by sysctl */ -#define FS_MQUEUE 1 -#define CTL_QUEUESMAX 2 -#define CTL_MSGMAX 3 -#define CTL_MSGSIZEMAX 4 -/* default values */ #define DFLT_QUEUESMAX 256/* max number of message queues */ #define DFLT_MSGMAX 10/* max number of messages in each queue */
@@ -1197,7 +1191,6 @@ static int msg_maxsize_limit_max = INT_MAX;static ctl_table mq_sysctls[] = { { - .ctl_name =CTL_QUEUESMAX , .procname ="queues_max" , .data = &queues_max, .maxlen =sizeof (int ),
@@ -1205,7 +1198,6 @@ static ctl_table mq_sysctls[] = { .proc_handler = &proc_dointvec, }, { - .ctl_name = CTL_MSGMAX, .procname = "msg_max", .data = &msg_max, .maxlen = sizeof(int),
@@ -1215,7 +1207,6 @@ static ctl_table mq_sysctls[] = { .extra2 = &msg_max_limit_max, }, { - .ctl_name = CTL_MSGSIZEMAX, .procname = "msgsize_max", .data = &msgsize_max, .maxlen = sizeof(int),
@@ -1229,7 +1220,6 @@ static ctl_table mq_sysctls[] = {static ctl_table mq_sysctl_dir[] = { { - .ctl_name =FS_MQUEUE , .procname ="mqueue" , .mode =0555 , .child = mq_sysctls,
Signature · 1 line
1.5.1.1.181.g2de0
PATCH 3/10 sysctl: Remove binary sysctl support where it clearly doesn't work.
These functions all of wrapper functions for the proc interface that are needed for them to work correctly.
--- kernel/sysctl.c | 7 ------- 1 files changed, 0 insertions(+), 7 deletions(-)
kernel/sysctl.c +0 -7
@@ -350,7 +350,6 @@ static struct ctl_table kern_table[] = { },#ifdef CONFIG_PROC_SYSCTL { - .ctl_name = KERN_TAINTED, .procname = "tainted", .data = &tainted, .maxlen = sizeof(int),
@@ -359,7 +358,6 @@ static struct ctl_table kern_table[] = { },#endif { - .ctl_name = KERN_CAP_BSET, .procname = "cap-bound", .data = &cap_bset, .maxlen = sizeof(kernel_cap_t),
@@ -635,7 +633,6 @@ static struct ctl_table kern_table[] = { .proc_handler = &proc_dointvec, }, { - .ctl_name = KERN_NMI_WATCHDOG, .procname = "nmi_watchdog", .data = &nmi_watchdog_enabled, .maxlen = sizeof (int),
@@ -818,7 +815,6 @@ static struct ctl_table vm_table[] = { .extra2 = &one_hundred, }, { - .ctl_name = VM_DIRTY_WB_CS, .procname = "dirty_writeback_centisecs", .data = &dirty_writeback_interval, .maxlen = sizeof(dirty_writeback_interval),
@@ -826,7 +822,6 @@ static struct ctl_table vm_table[] = { .proc_handler = &dirty_writeback_centisecs_handler, }, { - .ctl_name = VM_DIRTY_EXPIRE_CS, .procname = "dirty_expire_centisecs", .data = &dirty_expire_interval, .maxlen = sizeof(dirty_expire_interval),
@@ -854,7 +849,6 @@ static struct ctl_table vm_table[] = { },#ifdef CONFIG_HUGETLB_PAGE { - .ctl_name = VM_HUGETLB_PAGES, .procname = "nr_hugepages", .data = &max_huge_pages, .maxlen = sizeof(unsigned long),
@@ -1079,7 +1073,6 @@ static struct ctl_table fs_table[] = { .proc_handler = &proc_dointvec, }, { - .ctl_name = FS_NRFILE, .procname = "file-nr", .data = &files_stat, .maxlen = 3*sizeof(int),
Signature · 1 line
1.5.1.1.181.g2de0
PATCH 4/10 sysctl: Fix neighbour table sysctls.
- In ipv6 ndisc_ifinfo_syctl_change so it doesn't depend on binary sysctl names for a function that works with proc.
- In neighbour.c reorder the table to put the possibly unused entries at the end so we can remove them by terminating the table early.
- In neighbour.c kill the entries with questionable binary sysctl handling behavior.
- In neighbour.c if we don't have a strategy routine remove the binary path. So we don't the default sysctl strategy routine on data that is not ready for it.
--- net/core/neighbour.c | 75 ++++++++++++++++++++++++++------------------------ net/ipv6/ndisc.c | 24 ++++++--------- 2 files changed, 49 insertions(+), 50 deletions(-)
net/core/neighbour.c +39 -36
@@ -2498,7 +2498,6 @@ static struct neigh_sysctl_table { .proc_handler = &proc_dointvec, }, { - .ctl_name = NET_NEIGH_RETRANS_TIME, .procname = "retrans_time", .maxlen = sizeof(int), .mode = 0644,
@@ -2543,27 +2542,40 @@ static struct neigh_sysctl_table { .proc_handler = &proc_dointvec, }, { - .ctl_name = NET_NEIGH_ANYCAST_DELAY, .procname = "anycast_delay", .maxlen = sizeof(int), .mode = 0644, .proc_handler = &proc_dointvec_userhz_jiffies, }, { - .ctl_name = NET_NEIGH_PROXY_DELAY, .procname = "proxy_delay", .maxlen = sizeof(int), .mode = 0644, .proc_handler = &proc_dointvec_userhz_jiffies, }, { - .ctl_name = NET_NEIGH_LOCKTIME, .procname = "locktime", .maxlen = sizeof(int), .mode = 0644, .proc_handler = &proc_dointvec_userhz_jiffies, }, { + .ctl_name = NET_NEIGH_RETRANS_TIME_MS, + .procname = "retrans_time_ms", + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = &proc_dointvec_ms_jiffies, + .strategy = &sysctl_ms_jiffies, + }, + { + .ctl_name = NET_NEIGH_REACHABLE_TIME_MS, + .procname = "base_reachable_time_ms", + .maxlen = sizeof(int), + .mode = 0644, + .proc_handler = &proc_dointvec_ms_jiffies, + .strategy = &sysctl_ms_jiffies, + }, + { .ctl_name = NET_NEIGH_GC_INTERVAL, .procname = "gc_interval", .maxlen = sizeof(int),
@@ -2592,22 +2604,7 @@ static struct neigh_sysctl_table { .mode = 0644, .proc_handler = &proc_dointvec, }, - { - .ctl_name = NET_NEIGH_RETRANS_TIME_MS, - .procname = "retrans_time_ms", - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = &proc_dointvec_ms_jiffies, - .strategy = &sysctl_ms_jiffies, - }, - { - .ctl_name = NET_NEIGH_REACHABLE_TIME_MS, - .procname = "base_reachable_time_ms", - .maxlen = sizeof(int), - .mode = 0644, - .proc_handler = &proc_dointvec_ms_jiffies, - .strategy = &sysctl_ms_jiffies, - }, + {} }, .neigh_dev = { {
@@ -2660,42 +2657,48 @@ int neigh_sysctl_register(struct net_device *dev, struct neigh_parms *p, t->neigh_vars[9 ].data = &p->anycast_delay; t->neigh_vars[10 ].data = &p->proxy_delay; t->neigh_vars[11 ].data = &p->locktime; + t->neigh_vars[12 ].data = &p->retrans_time; + t->neigh_vars[13 ].data = &p->base_reachable_time;if (dev) { dev_name_source = dev->name; t->neigh_dev[0 ].ctl_name = dev->ifindex; - t->neigh_vars[12 ].procname =NULL ; - t->neigh_vars[13 ].procname =NULL ; - t->neigh_vars[14 ].procname =NULL ; - t->neigh_vars[15 ].procname =NULL ; +/* Terminate the table early */ +memset (&t->neigh_vars[14 ],0 ,sizeof (t->neigh_vars[14 ])); }else { dev_name_source = t->neigh_dev[0 ].procname; - t->neigh_vars[12 ].data = (int *)(p +1 ); - t->neigh_vars[13 ].data = (int *)(p +1 ) +1 ; - t->neigh_vars[14 ].data = (int *)(p +1 ) +2 ; - t->neigh_vars[15 ].data = (int *)(p +1 ) +3 ; + t->neigh_vars[14 ].data = (int *)(p +1 ); + t->neigh_vars[15 ].data = (int *)(p +1 ) +1 ; + t->neigh_vars[16 ].data = (int *)(p +1 ) +2 ; + t->neigh_vars[17 ].data = (int *)(p +1 ) +3 ; } - t->neigh_vars[16 ].data = &p->retrans_time; - t->neigh_vars[17 ].data = &p->base_reachable_time;if (handler || strategy) {/* RetransTime */ t->neigh_vars[3 ].proc_handler = handler; t->neigh_vars[3 ].strategy = strategy; t->neigh_vars[3 ].extra1 = dev; +if (!strategy) + t->neigh_vars[3 ].ctl_name =CTL_UNNUMBERED ;/* ReachableTime */ t->neigh_vars[4 ].proc_handler = handler; t->neigh_vars[4 ].strategy = strategy; t->neigh_vars[4 ].extra1 = dev; +if (!strategy) + t->neigh_vars[4 ].ctl_name =CTL_UNNUMBERED ;/* RetransTime (in milliseconds)*/ - t->neigh_vars[16 ].proc_handler = handler; - t->neigh_vars[16 ].strategy = strategy; - t->neigh_vars[16 ].extra1 = dev; + t->neigh_vars[12 ].proc_handler = handler; + t->neigh_vars[12 ].strategy = strategy; + t->neigh_vars[12 ].extra1 = dev; +if (!strategy) + t->neigh_vars[12 ].ctl_name =CTL_UNNUMBERED ;/* ReachableTime (in milliseconds) */ - t->neigh_vars[17 ].proc_handler = handler; - t->neigh_vars[17 ].strategy = strategy; - t->neigh_vars[17 ].extra1 = dev; + t->neigh_vars[13 ].proc_handler = handler; + t->neigh_vars[13 ].strategy = strategy; + t->neigh_vars[13 ].extra1 = dev; +if (!strategy) + t->neigh_vars[13 ].ctl_name =CTL_UNNUMBERED ; } dev_name =kstrdup (dev_name_source,GFP_KERNEL );
net/ipv6/ndisc.c +10 -14
@@ -1570,30 +1570,26 @@ int ndisc_ifinfo_sysctl_change(struct ctl_table *ctl, int write, struct file * fstruct inet6_dev *idev;int ret; -if (ctl->ctl_name ==NET_NEIGH_RETRANS_TIME || - ctl->ctl_name ==NET_NEIGH_REACHABLE_TIME ) +if ((strcmp (ctl->procname,"retrans_time" ) ==0 ) || + (strcmp (ctl->procname,"base_reachable_time" ) ==0 ))ndisc_warn_deprecated_sysctl (ctl,"syscall" , dev ? dev->name :"default" ); -switch (ctl->ctl_name) { -case NET_NEIGH_RETRANS_TIME : +if (strcmp (ctl->procname,"retrans_time" ) ==0 ) ret =proc_dointvec (ctl, write, filp, buffer, lenp, ppos); -break ; -case NET_NEIGH_REACHABLE_TIME : + +else if (strcmp (ctl->procname,"base_reachable_time" ) ==0 ) ret =proc_dointvec_jiffies (ctl, write, filp, buffer, lenp, ppos); -break ; -case NET_NEIGH_RETRANS_TIME_MS : -case NET_NEIGH_REACHABLE_TIME_MS : + +else if ((strcmp (ctl->procname,"retrans_time_ms" ) ==0 ) || + (strcmp (ctl->procname,"base_reacable_time_ms" ) ==0 )) ret =proc_dointvec_ms_jiffies (ctl, write, filp, buffer, lenp, ppos); -break ; -default : +else ret =-1 ; - }if (write && ret ==0 && dev && (idev =in6_dev_get (dev)) !=NULL ) { -if (ctl->ctl_name ==NET_NEIGH_REACHABLE_TIME || - ctl->ctl_name ==NET_NEIGH_REACHABLE_TIME_MS ) +if (ctl->data == &idev->nd_parms->base_reachable_time) idev->nd_parms->reachable_time =neigh_rand_reach_time (idev->nd_parms->base_reachable_time); idev->tstamp = jiffies;inet6_ifinfo_notify (RTM_NEWLINK , idev);
Signature · 1 line
1.5.1.1.181.g2de0
PATCH 5/10 sysctl: ipv6 route flushing (kill binary path)
We don't preoperly support the sysctl binary path for flushing the ipv6 routes. So remove support for a binary path.
--- net/ipv6/route.c | 1 - 1 files changed, 0 insertions(+), 1 deletions(-)
net/ipv6/route.c +0 -1
@@ -2458,7 +2458,6 @@ int ipv6_sysctl_rtcache_flush(ctl_table *ctl, int write, struct file * filp,ctl_table ipv6_route_table[] = { { - .ctl_name =NET_IPV6_ROUTE_FLUSH , .procname ="flush" , .data = &flush_delay, .maxlen =sizeof (int ),
Signature · 1 line
1.5.1.1.181.g2de0
PATCH 6/10 sysctl: Remove broken sunrpc debug binary sysctls
Quoted 1 line
From
ddf280c9de903f1fb5d4ecf9c68df0c479d7c7d2Mon Sep 17 00:00:00 2001
From: Eric W. Biederman <[email protected]>
Date: Thu, 9 Aug 2007 16:00:00 -0600
Subject:
This is debug code so no need to support binary sysctl, and the binary sysctls as they were written were not consistent with what showed up in /proc so remove the binary sysctl support.
--- net/sunrpc/sysctl.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-)
net/sunrpc/sysctl.c +0 -4
@@ -114,7 +114,6 @@ done:static ctl_table debug_table[] = { { - .ctl_name =CTL_RPCDEBUG , .procname ="rpc_debug" , .data = &rpc_debug, .maxlen =sizeof (int ),
@@ -122,7 +121,6 @@ static ctl_table debug_table[] = { .proc_handler = &proc_dodebug }, { - .ctl_name = CTL_NFSDEBUG, .procname = "nfs_debug", .data = &nfs_debug, .maxlen = sizeof(int),
@@ -130,7 +128,6 @@ static ctl_table debug_table[] = { .proc_handler = &proc_dodebug }, { - .ctl_name = CTL_NFSDDEBUG, .procname = "nfsd_debug", .data = &nfsd_debug, .maxlen = sizeof(int),
@@ -138,7 +135,6 @@ static ctl_table debug_table[] = { .proc_handler = &proc_dodebug }, { - .ctl_name = CTL_NLMDEBUG, .procname = "nlm_debug", .data = &nlm_debug, .maxlen = sizeof(int),
Signature · 1 line
1.5.1.1.181.g2de0
PATCH 7/10 sysctl: x86_64 remove unnecessary binary paths.
--- arch/x86_64/ia32/ia32_binfmt.c | 1 - arch/x86_64/kernel/vsyscall.c | 10 +--------- 2 files changed, 1 insertions(+), 10 deletions(-)
arch/x86_64/ia32/ia32_binfmt.c +0 -1
@@ -291,7 +291,6 @@ static void elf32_init(struct pt_regs *regs)static ctl_table abi_table2[] = { { - .ctl_name =99 , .procname ="vsyscall32" , .data = &sysctl_vsyscall32, .maxlen =sizeof (int ),
arch/x86_64/kernel/vsyscall.c +1 -9
@@ -260,18 +260,10 @@ out:return ret; } -static int vsyscall_sysctl_nostrat (ctl_table *t,int __user *name,int nlen, -void __user *oldval,size_t __user *oldlenp, -void __user *newval,size_t newlen) -{ -return -ENOSYS ; -} -static ctl_table kernel_table2[] = { - { .ctl_name =99 , .procname ="vsyscall64" , + { .procname ="vsyscall64" , .data = &vsyscall_gtod_data.sysctl_enabled, .maxlen =sizeof (int ), .mode =0644 , - .strategy = vsyscall_sysctl_nostrat, .proc_handler = vsyscall_sysctl_change }, {} };
Signature · 1 line
1.5.1.1.181.g2de0
PATCH 8/10 sysctl: Remove broken cdrom binary sysctls
The binary interface for the cdrom sysctls can't possilby work. So remove the binary sysctls and update the test for finding out which sysctl table entry we are dealy with to use the procname and not the ctl_name (which I am removing).
--- drivers/cdrom/cdrom.c | 31 +++++++++---------------------- 1 files changed, 9 insertions(+), 22 deletions(-)
drivers/cdrom/cdrom.c +9 -22
@@ -3468,32 +3468,25 @@ static int cdrom_sysctl_handler(ctl_table *ctl, int write, struct file * filp,else *valp =0 ; -switch (ctl->ctl_name) { -case DEV_CDROM_AUTOCLOSE : { +if (strcmp (ctl->procname,"autoclose" ) ==0 ) {if (valp == &cdrom_sysctl_settings.autoclose) autoclose = cdrom_sysctl_settings.autoclose; -break ; - } -case DEV_CDROM_AUTOEJECT : { + } +else if (strcmp (ctl->procname,"autoeject" ) ==0 ) {if (valp == &cdrom_sysctl_settings.autoeject) autoeject = cdrom_sysctl_settings.autoeject; -break ; - } -case DEV_CDROM_DEBUG : { + } +else if (strcmp (ctl->procname,"debug" ) ==0 ) {if (valp == &cdrom_sysctl_settings.debug) debug = cdrom_sysctl_settings.debug; -break ; - } -case DEV_CDROM_LOCK : { + } +else if (strcmp (ctl->procname,"lock" ) ==0 ) {if (valp == &cdrom_sysctl_settings.lock) lockdoor = cdrom_sysctl_settings.lock; -break ; - } -case DEV_CDROM_CHECK_MEDIA : { + } +else if (strcmp (ctl->procname,"check_media" ) ==0 ) {if (valp == &cdrom_sysctl_settings.check) check_media_type = cdrom_sysctl_settings.check; -break ; - } }/* update the option flags according to the changes. we don't have per device options through sysctl yet,
@@ -3507,7 +3500,6 @@ static int cdrom_sysctl_handler(ctl_table *ctl, int write, struct file * filp,/* Place files in /proc/sys/dev/cdrom */ static ctl_table cdrom_table[] = { { - .ctl_name =DEV_CDROM_INFO , .procname ="info" , .data = &cdrom_sysctl_settings.info, .maxlen =CDROM_STR_SIZE ,
@@ -3515,7 +3507,6 @@ static ctl_table cdrom_table[] = { .proc_handler = &cdrom_sysctl_info, }, { - .ctl_name = DEV_CDROM_AUTOCLOSE, .procname = "autoclose", .data = &cdrom_sysctl_settings.autoclose, .maxlen = sizeof(int),
@@ -3523,7 +3514,6 @@ static ctl_table cdrom_table[] = { .proc_handler = &cdrom_sysctl_handler, }, { - .ctl_name = DEV_CDROM_AUTOEJECT, .procname = "autoeject", .data = &cdrom_sysctl_settings.autoeject, .maxlen = sizeof(int),
@@ -3531,7 +3521,6 @@ static ctl_table cdrom_table[] = { .proc_handler = &cdrom_sysctl_handler, }, { - .ctl_name = DEV_CDROM_DEBUG, .procname = "debug", .data = &cdrom_sysctl_settings.debug, .maxlen = sizeof(int),
@@ -3539,7 +3528,6 @@ static ctl_table cdrom_table[] = { .proc_handler = &cdrom_sysctl_handler, }, { - .ctl_name = DEV_CDROM_LOCK, .procname = "lock", .data = &cdrom_sysctl_settings.lock, .maxlen = sizeof(int),
@@ -3547,7 +3535,6 @@ static ctl_table cdrom_table[] = { .proc_handler = &cdrom_sysctl_handler, }, { - .ctl_name = DEV_CDROM_CHECK_MEDIA, .procname = "check_media", .data = &cdrom_sysctl_settings.check, .maxlen = sizeof(int),
Signature · 1 line
1.5.1.1.181.g2de0
PATCH 9/10 sysctl: ipv4 remove binary sysctl paths where they are broken.
Currently tcp_available_congestion_control does not even attempt being read from sys_sysctl, and ipfrag_max_dist while it works allows setting of invalid values using sys_sysctl.
So just kill the binary sys_sysctl support for these sysctls. If the support is not important enough to test and get right it probably isn't important enough to keep.
--- net/ipv4/sysctl_net_ipv4.c | 2 -- 1 files changed, 0 insertions(+), 2 deletions(-)
net/ipv4/sysctl_net_ipv4.c +0 -2
@@ -672,7 +672,6 @@ ctl_table ipv4_table[] = { .strategy = &sysctl_jiffies }, { - .ctl_name = NET_IPV4_IPFRAG_MAX_DIST, .procname = "ipfrag_max_dist", .data = &sysctl_ipfrag_max_dist, .maxlen = sizeof(int),
@@ -797,7 +796,6 @@ ctl_table ipv4_table[] = { },#endif /* CONFIG_NETLABEL */ { - .ctl_name = NET_TCP_AVAIL_CONG_CONTROL, .procname = "tcp_available_congestion_control", .maxlen = TCP_CA_BUF_MAX, .mode = 0444,
Signature · 1 line
1.5.1.1.181.g2de0
PATCH 10/10 sysctl: Remove the binary interface for aio-nr, aio-max-nr, acpi_video_flags
aio-nr, aio-max-nr, acpi_video_flags are unsigned long values which sysctl does not handle properly with a 64bit kernel and a 32bit user space.
Since no one is likely to be using the binary sysctl values and the ascii interface still works, this patch just removes support for the binary sysctl interface from the kernel.
--- kernel/sysctl.c | 3 --- 1 files changed, 0 insertions(+), 3 deletions(-)
kernel/sysctl.c +0 -3
@@ -688,7 +688,6 @@ static struct ctl_table kern_table[] = {#endif #if defined(CONFIG_ACPI_SLEEP ) && defined(CONFIG_X86 ) { - .ctl_name = KERN_ACPI_VIDEO_FLAGS, .procname = "acpi_video_flags", .data = &acpi_realmode_flags, .maxlen = sizeof (unsigned long),
@@ -1148,7 +1147,6 @@ static struct ctl_table fs_table[] = { .extra2 = &two, }, { - .ctl_name = FS_AIO_NR, .procname = "aio-nr", .data = &aio_nr, .maxlen = sizeof(aio_nr),
@@ -1156,7 +1154,6 @@ static struct ctl_table fs_table[] = { .proc_handler = &proc_doulongvec_minmax, }, { - .ctl_name = FS_AIO_MAX_NR, .procname = "aio-max-nr", .data = &aio_max_nr, .maxlen = sizeof(aio_max_nr),
Signature · 1 line
1.5.1.1.181.g2de0
Alan Cox
NAKRe: [PATCH 08/10] sysctl: Remove broken cdrom binary sysctls
In reply to Eric W. BiedermanQuoted 5 lines · On Thu, 09 Aug 2007 19:01:14 -0600 [email protected] (Eric W. Biederman)…
The binary interface for the cdrom sysctls can't possilby work. So remove the binary sysctls and update the test for finding out which sysctl table entry we are dealy with to use the procname and not the
ctl_name(which I am removing).
NAK
I've no problem with the basic idea of removing the names from user space, but you've just demonstrated with the patch below that it causes some quite revolting changes for the bad in code which previously was nicely done switch statements and is now a nest of string compares and inline code.
There are very good reasons to have a cookie in the sysctl entry for internal use and which is private to the subsystem using the sysctl (ie not for sysctl() use and not neccessarily unique).
Re: [PATCH 08/10] sysctl: Remove broken cdrom binary sysctls
Quoted 17 lines · Alan Cox
Quoted 5 lines · On Thu, 09 Aug 2007 19:01:14 -0600 [email protected] (Eric W. Biederman)…
The binary interface for the cdrom sysctls can't possilby work. So remove the binary sysctls and update the test for finding out which sysctl table entry we are dealy with to use the procname and not the
ctl_name(which I am removing).NAK
I've no problem with the basic idea of removing the names from user space, but you've just demonstrated with the patch below that it causes some quite revolting changes for the bad in code which previously was nicely done switch statements and is now a nest of string compares and inline code.
Quoted 3 lines
There are very good reasons to have a cookie in the sysctl entry for internal use and which is private to the subsystem using the sysctl (ie not for
sysctl()use and not neccessarily unique).
Sure. We currently have 3 fields for that: data, extra1, and extra2. You need to cast them to get an integer values but their definition especially extra1 and extra2 are left up to the caller.
In that code we could just do a test on valp (i.e. data in struct ctl_table).
I just wanted something stupid and simple, so the -mm tree would not be full of broken sysctls.
At least to my eyes the code doesn't look much worse. But if you want to do something better feel free.
Eric
Re: [PATCH 08/10] sysctl: Remove broken cdrom binary sysctls
Quoted 2 lines
In that code we could just do a test on valp (i.e. data in struct
ctl_table).
Or you could pass the struct pointer and subtract the array start
Quoted 2 lines
I just wanted something stupid and simple, so the -mm tree would not be full of broken sysctls.
Well I'd like you to not replace one pile of hard to clean up but easy to locate crap by smearing other hard to fix in one go crap over the kernel.
Quoted 3 lines
At least to my eyes the code doesn't look much worse. But if you want to do something better feel free.
I'll add it to my todo list. But in the meantime I still NAK the change. I'll try and get a patch sorted but it'll be after the kernel summit for sure.
Re: [PATCH 08/10] sysctl: Remove broken cdrom binary sysctls
Quoted 5 lines · Alan Cox
Quoted 2 lines
I just wanted something stupid and simple, so the -mm tree would not be full of broken sysctls.
Well I'd like you to not replace one pile of hard to clean up but easy to locate crap by smearing other hard to fix in one go crap over the kernel.
I'm not certain I see the difference. I just replaced the comparison of one label with another. There may have been more beauty in one or the other but it really should not have made a difference maintenance wise. Perhaps I'm just short some cdrom context here to understand your problem.
Regardless. Looking at the code I think we can do something straightforward and even more stupid without needing any comparisons.
Alan does this look a little better?
static int cdrom_sysctl_handler(struct ctl_table *ctl, int write, struct file * filp,
void __user *buffer, size_t *lenp, loff_t *ppos)
{
int *valp = ctl->data;
int val = *valp;
int ret;
ret = proc_dointvec(ctl, write, filp, buffer, lenp, ppos);
if (write) {
/* we only care for 1 or 0. */
autoclose = !!cdrom_sysctl_settings.autoclose;
autoeject = !!cdrom_sysctl_settings.autoeject;
debug = !!cdrom_sysctl_settings.debug;
lockdoor = !!cdrom_sysctl_settings.lock;
check_media_type = !!cdrom_sysctl_settings.check;
/* update the option flags according to the changes. we
don't have per device options through sysctl yet,
but we will have and then this will disappear. */
cdrom_update_settings();
}
return ret;
}Eric
PATCH 6/10 sysctl: Remove broken sunrpc debug binary sysctls
In reply to Eric W. BiedermanThis is debug code so no need to support binary sysctl, and the binary sysctls as they were written were not consistent with what showed up in /proc so remove the binary sysctl support.
--- net/sunrpc/sysctl.c | 4 ---- 1 files changed, 0 insertions(+), 4 deletions(-)
net/sunrpc/sysctl.c +0 -4
@@ -114,7 +114,6 @@ done:static ctl_table debug_table[] = { { - .ctl_name =CTL_RPCDEBUG , .procname ="rpc_debug" , .data = &rpc_debug, .maxlen =sizeof (int ),
@@ -122,7 +121,6 @@ static ctl_table debug_table[] = { .proc_handler = &proc_dodebug }, { - .ctl_name = CTL_NFSDEBUG, .procname = "nfs_debug", .data = &nfs_debug, .maxlen = sizeof(int),
@@ -130,7 +128,6 @@ static ctl_table debug_table[] = { .proc_handler = &proc_dodebug }, { - .ctl_name = CTL_NFSDDEBUG, .procname = "nfsd_debug", .data = &nfsd_debug, .maxlen = sizeof(int),
@@ -138,7 +135,6 @@ static ctl_table debug_table[] = { .proc_handler = &proc_dodebug }, { - .ctl_name = CTL_NLMDEBUG, .procname = "nlm_debug", .data = &nlm_debug, .maxlen = sizeof(int),
Signature · 1 line
1.5.1.1.181.g2de0
Re: [PATCH 04/10] sysctl: Fix neighbour table sysctls.
In reply to Eric W. BiedermanHello.
In article <[email protected]> (at Thu, 09 Aug 2007 18:56:09 -0600), [email protected] (Eric W. Biederman) says:
Quoted 7 lines
- In ipv6
ndisc_ifinfo_syctl_changeso it doesn't depend on binary sysctl names for a function that works with proc.- In
neighbour.creorder the table to put the possibly unused entries at the end so we can remove them by terminating the table early.
Quoted 7 lines
- In
neighbour.ckill the entries with questionable binary sysctl handling behavior.- In
neighbour.cif we don't have a strategy routine remove the binary path. So we don't the default sysctl strategy routine on data that is not ready for it.
I disagree. It is bad to remove existing interface. Ditto for other patches.
Regards,
--yoshfuji
David Miller
maintainerRe: [PATCH 04/10] sysctl: Fix neighbour table sysctls.
From: YOSHIFUJI Hideaki / 吉藤英明 <[email protected]>
Date: Fri, 10 Aug 2007 10:47:10 +0900 (JST)
Quoted 2 lines
I disagree. It is bad to remove existing interface. Ditto for other patches.
I think perhaps you misunderstand what Eric is doing.
sys_sysctl() isn't working properly for these cases and it is both a deprecated interface and not worth the pain of adding support in these cases.
The fact that nobody complains that none of this stuff works via sys_sysctl() to me proves that it is never used.
Re: [PATCH 04/10] sysctl: Fix neighbour table sysctls.
In article <[email protected]> (at Thu, 09 Aug 2007 18:49:21 -0700 (PDT)), David Miller <[email protected]> says:
Quoted 11 lines
From: YOSHIFUJI Hideaki / 吉藤英明 <[email protected]>
Date: Fri, 10 Aug 2007 10:47:10 +0900 (JST)Quoted 2 lines
I disagree. It is bad to remove existing interface. Ditto for other patches.
I think perhaps you misunderstand what Eric is doing.
sys_sysctl()isn't working properly for these cases and it is both a deprecated interface and not worth the pain of adding support in these cases.
Would you explain why it does not work properly
for those cases?
--yoshfuji
Re: [PATCH 04/10] sysctl: Fix neighbour table sysctls.
Quoted 2 lines · YOSHIFUJI Hideaki / 吉藤英明
Would you explain why it does not work properly
for those cases?
Mostly no appropriate strategy routine was setup to report the data to the caller of sys_sysctl.
Eric
Re: [PATCH 04/10] sysctl: Fix neighbour table sysctls.
In article <[email protected]> (at Thu, 09 Aug 2007 20:23:16 -0600), [email protected] (Eric W. Biederman) says:
Quoted 7 lines
Quoted 2 lines · YOSHIFUJI Hideaki / 吉藤英明
Would you explain why it does not work properly
for those cases?Mostly no appropriate strategy routine was setup to report the data to the caller of
sys_sysctl.
I assume that default strategy have been existing for it, no?! Maybe, I do miss something...
--yoshfuji
Re: [PATCH 04/10] sysctl: Fix neighbour table sysctls.
Quoted 5 lines · YOSHIFUJI Hideaki / 吉藤英明
In article <[email protected]> (at Thu, 09 Aug 2007 20:23:16 -0600), [email protected] (Eric W. Biederman) says:
Quoted 2 lines
YOSHIFUJI Hideaki / 吉藤英明 <[email protected]> writes:
Quoted 8 lines
Quoted 5 lines
Quoted 2 lines
Would you explain why it does not work properly
for those cases?Mostly no appropriate strategy routine was setup to report the data to the caller of
sys_sysctl.I assume that default strategy have been existing for it, no?! Maybe, I do miss something...
I'd have to go through it case by case. But in general unless your proc_handler is proc_dointvec the default strategy routine which does a raw binary copy of your data out will generally do the wrong thing.
So especially if your data is jiffies or otherwise needs processing you don't want to use the default strategy routine.
Until relatively recently no one was really policing the sysctl interfaces and even now it isn't too serious.
Eric
Re: [PATCH 04/10] sysctl: Fix neighbour table sysctls.
In reply to YOSHIFUJI Hideaki / 吉藤英明Quoted 14 lines · On Fri, 10 Aug 2007 10:47:10 +0900 (JST) YOSHIFUJI Hideaki / 吉藤英明…
Hello.
In article <[email protected]> (at Thu, 09 Aug 2007 18:56:09 -0600), [email protected] (Eric W. Biederman) says:
Quoted 10 lines
- In ipv6
ndisc_ifinfo_syctl_changeso it doesn't depend on binary sysctl names for a function that works with proc.- In
neighbour.creorder the table to put the possibly unused entries at the end so we can remove them by terminating the table early.- In
neighbour.ckill the entries with questionable binary sysctl handling behavior.
Quoted 6 lines
Quoted 4 lines
- In
neighbour.cif we don't have a strategy routine remove the binary path. So we don't the default sysctl strategy routine on data that is not ready for it.I disagree. It is bad to remove existing interface.
But it is good to remove bad interfaces, if we possibly can.
It is worth making the attempt. Does anyone know of anything which will break? I fed NET_NEIGH_ANYCAST_DELAY at random into
http://www.google.com/codesearch and came up with nothing...
Re: [PATCH 04/10] sysctl: Fix neighbour table sysctls.
Quoted 5 lines · Andrew Morton
But it is good to remove bad interfaces, if we possibly can.
It is worth making the attempt. Does anyone know of anything which will break? I fed
NET_NEIGH_ANYCAST_DELAYat random into
http://www.google.com/codesearch and came up with nothing...
My current policy is that since I could only find 5 real world linux programs that even call sys_sysctl, that if I find a broken sysctl binary interface I'm lazy and just remove it. The only networking one I know of is radvd.
Added to that I just pushed an autochecking sysctl patch to Andrew that fails register_sysctl_table if the sysctl table is broken. And all of these showed up. So some fix was needed or things would have been even worse.
Eric
Re: [PATCH 04/10] sysctl: Fix neighbour table sysctls.
In reply to Eric W. BiedermanIn article <[email protected]> (at Thu, 09 Aug 2007 18:56:09 -0600), [email protected] (Eric W. Biederman) says:
Quoted 3 lines
- In ipv6
ndisc_ifinfo_syctl_changeso it doesn't depend on binary sysctl names for a function that works with proc.
:
Well, retrans_time_ms and base_reachable_time_ms supercedes retrans_time and base_reachable_time, we've warned for long time for its deprecation. So, maybe, it is time to remove the old interfaces (retrans_time and base_reachable_time) and simplify ndisc_ifinfo_syctl_change().
--yoshfuji
Re: [PATCH 3/3] sysctl: Error on bad sysctl tables
In reply to Eric W. BiedermanHello.
In article <[email protected]> (at Thu, 09 Aug 2007 14:09:29 -0600), [email protected] (Eric W. Biederman) says:
Quoted 5 lines
After going through the kernels sysctl tables several times it has become clear that code review and testing is just not effective in prevent problematic sysctl tables from being used in the stable kernel. I certainly can't seem to fix the problems as fast as they are introduced.
:
Quoted 4 lines
The biggest part of the code is the table of valid binary sysctl entries, but since we have frozen our set of binary sysctls this table should not need to change, and it makes it much easier to detect when someone unintentionally adds a new binary sysctl value.
I don't think everyone needs to have this code, so it is better to make it configurable via CONFIG_SYSCTL_DEBUG or something..., ...no?
--yoshfuji
Re: [PATCH 3/3] sysctl: Error on bad sysctl tables
Quoted 11 lines · YOSHIFUJI Hideaki / 吉藤英明
Hello.
In article <[email protected]> (at Thu, 09 Aug 2007 14:09:29 -0600), [email protected] (Eric W. Biederman) says:
Quoted 5 lines
After going through the kernels sysctl tables several times it has become clear that code review and testing is just not effective in prevent problematic sysctl tables from being used in the stable kernel. I certainly can't seem to fix the problems as fast as they are introduced.
:
Quoted 8 lines
Quoted 4 lines
The biggest part of the code is the table of valid binary sysctl entries, but since we have frozen our set of binary sysctls this table should not need to change, and it makes it much easier to detect when someone unintentionally adds a new binary sysctl value.
I don't think everyone needs to have this code, so it is better to make it configurable via
CONFIG_SYSCTL_DEBUGor something..., ...no?
I wouldn't reject such a patch. We are a ways out from the next stable kernel merge window and I'd love to see what else falls out so I'd like to have it on by default for a bit.
Eric
Re: [PATCH 3/3] sysctl: Error on bad sysctl tables
In reply to YOSHIFUJI Hideaki / 吉藤英明Quoted 11 lines · YOSHIFUJI Hideaki / 吉藤英明
Hello.
In article <[email protected]> (at Thu, 09 Aug 2007 14:09:29 -0600), [email protected] (Eric W. Biederman) says:
Quoted 5 lines
After going through the kernels sysctl tables several times it has become clear that code review and testing is just not effective in prevent problematic sysctl tables from being used in the stable kernel. I certainly can't seem to fix the problems as fast as they are introduced.
:
Quoted 8 lines
Quoted 4 lines
The biggest part of the code is the table of valid binary sysctl entries, but since we have frozen our set of binary sysctls this table should not need to change, and it makes it much easier to detect when someone unintentionally adds a new binary sysctl value.
I don't think everyone needs to have this code, so it is better to make it configurable via
CONFIG_SYSCTL_DEBUGor something..., ...no?
I guess the other thing is. Except for code size it doesn't matter. As register_sysctl_table gets called very rarely.
Eric