mbox series

[v3,0/2] Fixes multiple sysctl proc_handler usage error

Message ID 20241217132908.38096-1-nicolas.bouchinet@clip-os.org (mailing list archive)
Headers show
Series Fixes multiple sysctl proc_handler usage error | expand

Message

Nicolas Bouchinet Dec. 17, 2024, 1:29 p.m. UTC
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 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

 fs/coredump.c   | 4 +++-
 kernel/sysctl.c | 3 ++-
 2 files changed, 5 insertions(+), 2 deletions(-)