• Luis R. Rodriguez's avatar
    sysctl: simplify unsigned int support · 4f2fec00
    Luis R. Rodriguez authored
    Commit e7d316a0 ("sysctl: handle error writing UINT_MAX to u32
    fields") added proc_douintvec() to start help adding support for
    unsigned int, this however was only half the work needed.  Two fixes
    have come in since then for the following issues:
    
      o Printing the values shows a negative value, this happens since
        do_proc_dointvec() and this uses proc_put_long()
    
    This was fixed by commit 5380e564 ("sysctl: don't print negative
    flag for proc_douintvec").
    
      o We can easily wrap around the int values: UINT_MAX is 4294967295, if
        we echo in 4294967295 + 1 we end up with 0, using 4294967295 + 2 we
        end up with 1.
      o We echo negative values in and they are accepted
    
    This was fixed by commit 425fffd8 ("sysctl: report EINVAL if value
    is larger than UINT_MAX for proc_douintvec").
    
    It still also failed to be added to sysctl_check_table()...  instead of
    adding it with the current implementation just provide a proper and
    simplified unsigned int support without any array unsigned int support
    with no negative support at all.
    
    Historically sysctl proc helpers have supported arrays, due to the
    complexity this adds though we've taken a step back to evaluate array
    users to determine if its worth upkeeping for unsigned int.  An
    evaluation using Coccinelle has been done to perform a grammatical
    search to ask ourselves:
    
      o How many sysctl proc_dointvec() (int) users exist which likely
        should be moved over to proc_douintvec() (unsigned int) ?
    	Answer: about 8
    	- Of these how many are array users ?
    		Answer: Probably only 1
      o How many sysctl array users exist ?
    	Answer: about 12
    
    This last question gives us an idea just how popular arrays: they are not.
    Array support should probably just be kept for strings.
    
    The identified uint ports are:
    
      drivers/infiniband/core/ucma.c - max_backlog
      drivers/infiniband/core/iwcm.c - default_backlog
      net/core/sysctl_net_core.c - rps_sock_flow_sysctl()
      net/netfilter/nf_conntrack_timestamp.c - nf_conntrack_timestamp -- bool
      net/netfilter/nf_conntrack_acct.c nf_conntrack_acct -- bool
      net/netfilter/nf_conntrack_ecache.c - nf_conntrack_events -- bool
      net/netfilter/nf_conntrack_helper.c - nf_conntrack_helper -- bool
      net/phonet/sysctl.c proc_local_port_range()
    
    The only possible array users is proc_local_port_range() but it does not
    seem worth it to add array support just for this given the range support
    works just as well.  Unsigned int support should be desirable more for
    when you *need* more than INT_MAX or using int min/max support then does
    not suffice for your ranges.
    
    If you forget and by mistake happen to register an unsigned int proc
    entry with an array, the driver will fail and you will get something as
    follows:
    
    sysctl table check failed: debug/test_sysctl//uint_0002 array now allowed
    CPU: 2 PID: 1342 Comm: modprobe Tainted: G        W   E <etc>
    Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS <etc>
    Call Trace:
     dump_stack+0x63/0x81
     __register_sysctl_table+0x350/0x650
     ? kmem_cache_alloc_trace+0x107/0x240
     __register_sysctl_paths+0x1b3/0x1e0
     ? 0xffffffffc005f000
     register_sysctl_table+0x1f/0x30
     test_sysctl_init+0x10/0x1000 [test_sysctl]
     do_one_initcall+0x52/0x1a0
     ? kmem_cache_alloc_trace+0x107/0x240
     do_init_module+0x5f/0x200
     load_module+0x1867/0x1bd0
     ? __symbol_put+0x60/0x60
     SYSC_finit_module+0xdf/0x110
     SyS_finit_module+0xe/0x10
     entry_SYSCALL_64_fastpath+0x1e/0xad
    RIP: 0033:0x7f042b22d119
    <etc>
    
    Fixes: e7d316a0 ("sysctl: handle error writing UINT_MAX to u32 fields")
    Link: http://lkml.kernel.org/r/20170519033554.18592-5-mcgrof@kernel.orgSigned-off-by: default avatarLuis R. Rodriguez <mcgrof@kernel.org>
    Suggested-by: default avatarAlexey Dobriyan <adobriyan@gmail.com>
    Cc: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>
    Cc: Liping Zhang <zlpnobody@gmail.com>
    Cc: Alexey Dobriyan <adobriyan@gmail.com>
    Cc: Heinrich Schuchardt <xypron.glpk@gmx.de>
    Cc: Kees Cook <keescook@chromium.org>
    Cc: "David S. Miller" <davem@davemloft.net>
    Cc: Ingo Molnar <mingo@redhat.com>
    Cc: Al Viro <viro@zeniv.linux.org.uk>
    Cc: "Eric W. Biederman" <ebiederm@xmission.com>
    Signed-off-by: default avatarAndrew Morton <akpm@linux-foundation.org>
    Signed-off-by: default avatarLinus Torvalds <torvalds@linux-foundation.org>
    4f2fec00
sysctl.c 71.6 KB