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

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

Posted by Chao Huang 1 month, 2 weeks ago

From: Chao Huang <huangchao@kylinos.cn>

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 <huangchao@kylinos.cn>
---
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;
 	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);
+	}
 	kfree(device);
 	iput(inode);
 

base-commit: c21bb4193868a8de71fc4693fa741e195fdf5d86
-- 
2.25.1

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

Posted by Carlos Llamas 1 month, 2 weeks ago

On Mon, Aug 10, 2026 at 05:32:16PM +0800, Chao Huang wrote:
> From: Chao Huang <huangchao@kylinos.cn>
> 
> 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 <huangchao@kylinos.cn>
> ---
> 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
>

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

Posted by Chao Huang 1 month, 2 weeks ago

From: Chao Huang <huangchao@kylinos.cn>

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.

Release the minor from a dedicated error path after a successful allocation
in both implementations.

Signed-off-by: Chao Huang <huangchao@kylinos.cn>
---
Changes in v3:
- Use a dedicated out_with_minor error path instead of an -ENOSPC sentinel.

 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..0763e3db54bd 100644
--- a/drivers/android/binder/rust_binderfs.c
+++ b/drivers/android/binder/rust_binderfs.c
@@ -421,7 +421,7 @@ static int binderfs_binder_ctl_create(struct super_block *sb)
 
 	dentry = d_alloc_name(root, "binder-control");
 	if (!dentry)
-		goto out;
+		goto out_with_minor;
 
 	inode->i_private = device;
 	info->control_dentry = dentry;
@@ -430,6 +430,11 @@ static int binderfs_binder_ctl_create(struct super_block *sb)
 
 	return 0;
 
+out_with_minor:
+	mutex_lock(&binderfs_minors_mutex);
+	ida_free(&binderfs_minors, minor);
+	mutex_unlock(&binderfs_minors_mutex);
+
 out:
 	kfree(device);
 	iput(inode);
diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index 361d69f756f5..0e96146ecf69 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -431,7 +431,7 @@ static int binderfs_binder_ctl_create(struct super_block *sb)
 
 	dentry = d_alloc_name(root, "binder-control");
 	if (!dentry)
-		goto out;
+		goto out_with_minor;
 
 	inode->i_private = device;
 	info->control_dentry = dentry;
@@ -440,6 +440,11 @@ static int binderfs_binder_ctl_create(struct super_block *sb)
 
 	return 0;
 
+out_with_minor:
+	mutex_lock(&binderfs_minors_mutex);
+	ida_free(&binderfs_minors, minor);
+	mutex_unlock(&binderfs_minors_mutex);
+
 out:
 	kfree(device);
 	iput(inode);

base-commit: c21bb4193868a8de71fc4693fa741e195fdf5d86
-- 
2.25.1

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

Posted by Carlos Llamas 1 month, 2 weeks ago

On Tue, Aug 11, 2026 at 09:05:34AM +0800, Chao Huang wrote:
> From: Chao Huang <huangchao@kylinos.cn>
> 
> 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.
> 
> Release the minor from a dedicated error path after a successful allocation
> in both implementations.
> 
> Signed-off-by: Chao Huang <huangchao@kylinos.cn>
> ---
> Changes in v3:
> - Use a dedicated out_with_minor error path instead of an -ENOSPC sentinel.
> 
>  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..0763e3db54bd 100644
> --- a/drivers/android/binder/rust_binderfs.c
> +++ b/drivers/android/binder/rust_binderfs.c
> @@ -421,7 +421,7 @@ static int binderfs_binder_ctl_create(struct super_block *sb)
>  
>  	dentry = d_alloc_name(root, "binder-control");
>  	if (!dentry)
> -		goto out;
> +		goto out_with_minor;
>  
>  	inode->i_private = device;
>  	info->control_dentry = dentry;
> @@ -430,6 +430,11 @@ static int binderfs_binder_ctl_create(struct super_block *sb)
>  
>  	return 0;
>  
> +out_with_minor:
> +	mutex_lock(&binderfs_minors_mutex);
> +	ida_free(&binderfs_minors, minor);
> +	mutex_unlock(&binderfs_minors_mutex);
> +
>  out:
>  	kfree(device);
>  	iput(inode);
> diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> index 361d69f756f5..0e96146ecf69 100644
> --- a/drivers/android/binderfs.c
> +++ b/drivers/android/binderfs.c
> @@ -431,7 +431,7 @@ static int binderfs_binder_ctl_create(struct super_block *sb)
>  
>  	dentry = d_alloc_name(root, "binder-control");
>  	if (!dentry)
> -		goto out;
> +		goto out_with_minor;
>  
>  	inode->i_private = device;
>  	info->control_dentry = dentry;
> @@ -440,6 +440,11 @@ static int binderfs_binder_ctl_create(struct super_block *sb)
>  
>  	return 0;
>  
> +out_with_minor:
> +	mutex_lock(&binderfs_minors_mutex);
> +	ida_free(&binderfs_minors, minor);
> +	mutex_unlock(&binderfs_minors_mutex);
> +
>  out:
>  	kfree(device);
>  	iput(inode);
> 
> base-commit: c21bb4193868a8de71fc4693fa741e195fdf5d86
> -- 
> 2.25.1
> 

LGTM,

Acked-by: Carlos Llamas <cmllamas@google.com>