linux-kernel - Re: [PATCH v4 0/2] Fixes multiple sysctl proc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <unfyshpxhpvemwitht574ibizvuob55jgc45wbkiq7yvjvwvwq@ofo3kkmq5vvj>
Date: Mon, 17 Feb 2025 11:35:15 +0100
From: Joel Granados <joel.granados@...nel.org>
To: nicolas.bouchinet@...p-os.org
Cc: linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	Nicolas Bouchinet <nicolas.bouchinet@....gouv.fr>, Greg Kroah-Hartman <gregkh@...uxfoundation.org>, 
	Jiri Slaby <jirislaby@...nel.org>, Alexander Viro <viro@...iv.linux.org.uk>, 
	Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, Luis Chamberlain <mcgrof@...nel.org>, 
	Kees Cook <kees@...nel.org>, Joel Granados <j.granados@...sung.com>, 
	Andrew Morton <akpm@...ux-foundation.org>, Neil Horman <nhorman@...driver.com>, Lin Feng <linf@...gsu.com>, 
	Theodore Ts'o <tytso@....edu>
Subject: Re: [PATCH v4 0/2]  Fixes multiple sysctl proc_handler usage error

On Wed, Jan 15, 2025 at 02:22:07PM +0100, nicolas.bouchinet@...p-os.org wrote:
> From: Nicolas Bouchinet <nicolas.bouchinet@....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
> 

-- 

Joel Granados