Re: [PATCH v2] binderfs: free minor on binder-control creation failure

Carlos Llamas <[email protected]>
Newsgroups org.kernel.vger.linux-kernel
Message-ID <[email protected]>
On Mon, Aug 10, 2026 at 05:32:16PM +0800, Chao Huang wrote:
> From: Chao Huang <[email protected]>
> 
> Both binderfs_binder_ctl_create() implementations allocate a minor before
> creating the binder-control dentry. If d_alloc_name() fails, the error path
> frees the device and drops the inode, but leaves the minor allocated in
> binderfs_minors. Repeated failures can therefore exhaust the global minor
> IDA.
> 
> Initialize minor to an invalid value and release it from the common error
> path after a successful allocation in both implementations.
> 
> Signed-off-by: Chao Huang <[email protected]>
> ---
> Changes in v2:
> - Apply the same fix to drivers/android/binder/rust_binderfs.c.
> 
>  drivers/android/binder/rust_binderfs.c | 7 ++++++-
>  drivers/android/binderfs.c             | 7 ++++++-
>  2 files changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/android/binder/rust_binderfs.c b/drivers/android/binder/rust_binderfs.c
> index ade1c4d92499..c65a8e514986 100644
> --- a/drivers/android/binder/rust_binderfs.c
> +++ b/drivers/android/binder/rust_binderfs.c
> @@ -375,7 +375,7 @@ static const struct file_operations binder_ctl_fops = {
>   */
>  static int binderfs_binder_ctl_create(struct super_block *sb)
>  {
> -	int minor, ret;
> +	int minor = -ENOSPC, ret;
>  	struct dentry *dentry;
>  	struct binder_device *device;
>  	struct inode *inode = NULL;
> @@ -431,6 +431,11 @@ static int binderfs_binder_ctl_create(struct super_block *sb)
>  	return 0;
>  
>  out:
> +	if (minor >= 0) {
> +		mutex_lock(&binderfs_minors_mutex);
> +		ida_free(&binderfs_minors, minor);
> +		mutex_unlock(&binderfs_minors_mutex);
> +	}
>  	kfree(device);
>  	iput(inode);
>  
> diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> index 361d69f756f5..fdbf281d3418 100644
> --- a/drivers/android/binderfs.c
> +++ b/drivers/android/binderfs.c
> @@ -384,7 +384,7 @@ static const struct file_operations binder_ctl_fops = {
>   */
>  static int binderfs_binder_ctl_create(struct super_block *sb)
>  {
> -	int minor, ret;
> +	int minor = -ENOSPC, ret;

Why -ENOSPC? 

>  	struct dentry *dentry;
>  	struct binder_device *device;
>  	struct inode *inode = NULL;
> @@ -441,6 +441,11 @@ static int binderfs_binder_ctl_create(struct super_block *sb)
>  	return 0;
>  
>  out:
> +	if (minor >= 0) {
> +		mutex_lock(&binderfs_minors_mutex);
> +		ida_free(&binderfs_minors, minor);
> +		mutex_unlock(&binderfs_minors_mutex);
> +	}

How about a new "goto out_with_minor;" tag? This removes the odd ENOSPC
value and having to do this if (minor) check.

>  	kfree(device);
>  	iput(inode);
>  
> 
> base-commit: c21bb4193868a8de71fc4693fa741e195fdf5d86
> -- 
> 2.25.1
>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.