| Message ID | 20250115132211.25400-1-nicolas.bouchinet@clip-os.org (mailing list archive) |
|---|---|
| Headers | show |
| Series | Fixes multiple sysctl proc_handler usage error | expand |
On Wed, Jan 15, 2025 at 02:22:07PM +0100, nicolas.bouchinet@clip-os.org wrote: > From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr> > > Hi, while reading sysctl code I encountered two sysctl proc_handler > parameters common errors. > > The first one is to declare .data as a different type thant the return of > the used .proc_handler, i.e. using proch_dointvec, thats convert a char > string to signed integers, and storing the result in a .data that is backed > by an unsigned int. User can then write "-1" string, which results in a > different value stored in the .data variable. This can lead to type > conversion errors in branches and thus to potential security issues. > > From a quick search using regex and only for proc_dointvec, this seems to > be a pretty common mistake. > > The second one is to declare .extra1 or .extra2 values with a .proc_handler > that don't uses them. i.e, declaring .extra1 or .extra2 using proc_dointvec > in order to declare conversion bounds do not work as do_proc_dointvec don't > uses those variables if not explicitly asked. > > This patchset corrects three sysctl declaration that are buggy as an > example and is not exhaustive. After some time in sysctl-testing, pushing this to sysctl-next > > Nicolas > > --- > > Changes since v3: > https://lore.kernel.org/all/20241217132908.38096-1-nicolas.bouchinet@clip-os.org/ > > * Fixed patch 2/2 extra* parameter typo detected by Joel Granados. > * Reworded patch 2/2 as suggested by Joel Granados. > > > Changes since v2: > https://lore.kernel.org/all/20241114162638.57392-1-nicolas.bouchinet@clip-os.org/ > > * Bound vdso_enabled to 0 and 1 as suggested by Joel Granados. > * Remove patch 3/3 since Greg Kroah-Hartman merged it. > > Changes since v1: > https://lore.kernel.org/all/20241112131357.49582-1-nicolas.bouchinet@clip-os.org/ > > * Take Lin Feng review into account. > > --- > > Nicolas Bouchinet (2): > coredump: Fixes core_pipe_limit sysctl proc_handler > sysctl: Fix underflow value setting risk in vm_table > > arch/sh/kernel/vsyscall/vsyscall.c | 3 ++- > fs/coredump.c | 4 +++- > 2 files changed, 5 insertions(+), 2 deletions(-) > > -- > 2.48.1 >
From: Nicolas Bouchinet <nicolas.bouchinet@ssi.gouv.fr> Hi, while reading sysctl code I encountered two sysctl proc_handler parameters common errors. The first one is to declare .data as a different type thant the return of the used .proc_handler, i.e. using proch_dointvec, thats convert a char string to signed integers, and storing the result in a .data that is backed by an unsigned int. User can then write "-1" string, which results in a different value stored in the .data variable. This can lead to type conversion errors in branches and thus to potential security issues. From a quick search using regex and only for proc_dointvec, this seems to be a pretty common mistake. The second one is to declare .extra1 or .extra2 values with a .proc_handler that don't uses them. i.e, declaring .extra1 or .extra2 using proc_dointvec in order to declare conversion bounds do not work as do_proc_dointvec don't uses those variables if not explicitly asked. This patchset corrects three sysctl declaration that are buggy as an example and is not exhaustive. Nicolas --- Changes since v3: https://lore.kernel.org/all/20241217132908.38096-1-nicolas.bouchinet@clip-os.org/ * Fixed patch 2/2 extra* parameter typo detected by Joel Granados. * Reworded patch 2/2 as suggested by Joel Granados. Changes since v2: https://lore.kernel.org/all/20241114162638.57392-1-nicolas.bouchinet@clip-os.org/ * Bound vdso_enabled to 0 and 1 as suggested by Joel Granados. * Remove patch 3/3 since Greg Kroah-Hartman merged it. Changes since v1: https://lore.kernel.org/all/20241112131357.49582-1-nicolas.bouchinet@clip-os.org/ * Take Lin Feng review into account. --- Nicolas Bouchinet (2): coredump: Fixes core_pipe_limit sysctl proc_handler sysctl: Fix underflow value setting risk in vm_table arch/sh/kernel/vsyscall/vsyscall.c | 3 ++- fs/coredump.c | 4 +++- 2 files changed, 5 insertions(+), 2 deletions(-)