linux-kernel - Re: [PATCH] android: binder: print error message on failure of creating proc file

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <2024071214-onshore-moonbeam-4eeb@gregkh>
Date: Fri, 12 Jul 2024 07:40:37 +0200
From: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
To: Leesoo Ahn <lsahn@...eel.net>
Cc: Leesoo Ahn <lsahn@...akecorp.com>,
	Arve Hjønnevåg <arve@...roid.com>,
	Todd Kjos <tkjos@...roid.com>, Martijn Coenen <maco@...roid.com>,
	Joel Fernandes <joel@...lfernandes.org>,
	Christian Brauner <brauner@...nel.org>,
	Carlos Llamas <cmllamas@...gle.com>,
	Suren Baghdasaryan <surenb@...gle.com>,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] android: binder: print error message on failure of
 creating proc file

On Fri, Jul 12, 2024 at 12:21:40PM +0900, Leesoo Ahn wrote:
> It better prints out an error message to give more information if
> calling debugfs_create_file() is failure and the return value has an
> error code.
> 
> Signed-off-by: Leesoo Ahn <lsahn@...eel.net>
> ---
>  drivers/android/binder.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index b21a7b246a0d..eb0fd1443d69 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -5673,6 +5673,7 @@ static int binder_open(struct inode *nodp, struct file *filp)
>  
>  	if (binder_debugfs_dir_entry_proc && !existing_pid) {
>  		char strbuf[11];
> +		struct dentry *debugfs_entry;
>  
>  		snprintf(strbuf, sizeof(strbuf), "%u", proc->pid);
>  		/*
> @@ -5681,10 +5682,19 @@ static int binder_open(struct inode *nodp, struct file *filp)
>  		 * The printing code will anyway print all contexts for a given
>  		 * PID so this is not a problem.
>  		 */
> -		proc->debugfs_entry = debugfs_create_file(strbuf, 0444,
> +		debugfs_entry = debugfs_create_file(strbuf, 0444,
>  			binder_debugfs_dir_entry_proc,
>  			(void *)(unsigned long)proc->pid,
>  			&proc_fops);
> +		if (!IS_ERR(debugfs_entry)) {
> +			proc->debugfs_entry = debugfs_entry;
> +		} else {
> +			int error;
> +
> +			error = PTR_ERR(debugfs_entry);
> +			pr_warn("Unable to create file %s in debugfs (error %d)\n",
> +				strbuf, error);

Even if we wanted to warn about this (hint, you don't, see previous
response), this way to check is incorrect and will fail if debugfs is
not enabled, which you don't want to have happen.

So I'm guessing you did not test this with that config option disabled?

thanks,

greg k-h