[PATCH 1/2] binder: use kstrdup() in binderfs_binder_device_create()

[PATCH 1/2] binder: use kstrdup() in binderfs_binder_device_create()

[Dmitry Antipov]

In 'binderfs_binder_device_create()', use 'kstrdup()' to copy the
newly created device's name, thus making the former a bit simpler.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 drivers/android/binderfs.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index 024275dbfdd8..4f827152d18e 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -117,7 +117,6 @@ static int binderfs_binder_device_create(struct inode *ref_inode,
 	struct dentry *dentry, *root;
 	struct binder_device *device;
 	char *name = NULL;
-	size_t name_len;
 	struct inode *inode = NULL;
 	struct super_block *sb = ref_inode->i_sb;
 	struct binderfs_info *info = sb->s_fs_info;
@@ -161,9 +160,7 @@ static int binderfs_binder_device_create(struct inode *ref_inode,
 	inode->i_gid = info->root_gid;
 
 	req->name[BINDERFS_MAX_NAME] = '\0'; /* NUL-terminate */
-	name_len = strlen(req->name);
-	/* Make sure to include terminating NUL byte */
-	name = kmemdup(req->name, name_len + 1, GFP_KERNEL);
+	name = kstrdup(req->name, GFP_KERNEL);
 	if (!name)
 		goto err;
 
-- 
2.50.0

[PATCH 2/2] binder: use guards for plain mutex- and spinlock-protected sections

[Dmitry Antipov]

Use 'guard(mutex)' and 'guard(spinlock)' for plain (i.e. non-scoped)
mutex- and spinlock-protected sections, respectively, thus making
locking a bit simpler. Briefly tested with 'stress-ng --binderfs'.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
 drivers/android/binder.c       | 33 +++++++++++----------------------
 drivers/android/binder_alloc.c | 14 ++++----------
 drivers/android/binder_alloc.h |  8 ++------
 3 files changed, 17 insertions(+), 38 deletions(-)

diff --git a/drivers/android/binder.c b/drivers/android/binder.c
index c463ca4a8fff..b8a6a513e9b5 100644
--- a/drivers/android/binder.c
+++ b/drivers/android/binder.c
@@ -1585,11 +1585,10 @@ static struct binder_thread *binder_get_txn_from(
 {
 	struct binder_thread *from;
 
-	spin_lock(&t->lock);
+	guard(spinlock)(&t->lock);
 	from = t->from;
 	if (from)
 		atomic_inc(&from->tmp_ref);
-	spin_unlock(&t->lock);
 	return from;
 }
 
@@ -5445,32 +5444,28 @@ static int binder_ioctl_set_ctx_mgr(struct file *filp,
 	struct binder_node *new_node;
 	kuid_t curr_euid = current_euid();
 
-	mutex_lock(&context->context_mgr_node_lock);
+	guard(mutex)(&context->context_mgr_node_lock);
 	if (context->binder_context_mgr_node) {
 		pr_err("BINDER_SET_CONTEXT_MGR already set\n");
-		ret = -EBUSY;
-		goto out;
+		return -EBUSY;
 	}
 	ret = security_binder_set_context_mgr(proc->cred);
 	if (ret < 0)
-		goto out;
+		return ret;
 	if (uid_valid(context->binder_context_mgr_uid)) {
 		if (!uid_eq(context->binder_context_mgr_uid, curr_euid)) {
 			pr_err("BINDER_SET_CONTEXT_MGR bad uid %d != %d\n",
 			       from_kuid(&init_user_ns, curr_euid),
 			       from_kuid(&init_user_ns,
 					 context->binder_context_mgr_uid));
-			ret = -EPERM;
-			goto out;
+			return -EPERM;
 		}
 	} else {
 		context->binder_context_mgr_uid = curr_euid;
 	}
 	new_node = binder_new_node(proc, fbo);
-	if (!new_node) {
-		ret = -ENOMEM;
-		goto out;
-	}
+	if (!new_node)
+		return -ENOMEM;
 	binder_node_lock(new_node);
 	new_node->local_weak_refs++;
 	new_node->local_strong_refs++;
@@ -5479,8 +5474,6 @@ static int binder_ioctl_set_ctx_mgr(struct file *filp,
 	context->binder_context_mgr_node = new_node;
 	binder_node_unlock(new_node);
 	binder_put_node(new_node);
-out:
-	mutex_unlock(&context->context_mgr_node_lock);
 	return ret;
 }
 
@@ -6322,14 +6315,13 @@ static DECLARE_WORK(binder_deferred_work, binder_deferred_func);
 static void
 binder_defer_work(struct binder_proc *proc, enum binder_deferred_state defer)
 {
-	mutex_lock(&binder_deferred_lock);
+	guard(mutex)(&binder_deferred_lock);
 	proc->deferred_work |= defer;
 	if (hlist_unhashed(&proc->deferred_work_node)) {
 		hlist_add_head(&proc->deferred_work_node,
 				&binder_deferred_list);
 		schedule_work(&binder_deferred_work);
 	}
-	mutex_unlock(&binder_deferred_lock);
 }
 
 static void print_binder_transaction_ilocked(struct seq_file *m,
@@ -6871,14 +6863,13 @@ static int proc_show(struct seq_file *m, void *unused)
 	struct binder_proc *itr;
 	int pid = (unsigned long)m->private;
 
-	mutex_lock(&binder_procs_lock);
+	guard(mutex)(&binder_procs_lock);
 	hlist_for_each_entry(itr, &binder_procs, proc_node) {
 		if (itr->pid == pid) {
 			seq_puts(m, "binder proc state:\n");
 			print_binder_proc(m, itr, true, false);
 		}
 	}
-	mutex_unlock(&binder_procs_lock);
 
 	return 0;
 }
@@ -6996,16 +6987,14 @@ const struct binder_debugfs_entry binder_debugfs_entries[] = {
 
 void binder_add_device(struct binder_device *device)
 {
-	spin_lock(&binder_devices_lock);
+	guard(spinlock)(&binder_devices_lock);
 	hlist_add_head(&device->hlist, &binder_devices);
-	spin_unlock(&binder_devices_lock);
 }
 
 void binder_remove_device(struct binder_device *device)
 {
-	spin_lock(&binder_devices_lock);
+	guard(spinlock)(&binder_devices_lock);
 	hlist_del_init(&device->hlist);
-	spin_unlock(&binder_devices_lock);
 }
 
 static int __init init_binder_device(const char *name)
diff --git a/drivers/android/binder_alloc.c b/drivers/android/binder_alloc.c
index fcfaf1b899c8..a0a7cb58fc05 100644
--- a/drivers/android/binder_alloc.c
+++ b/drivers/android/binder_alloc.c
@@ -167,12 +167,8 @@ static struct binder_buffer *binder_alloc_prepare_to_free_locked(
 struct binder_buffer *binder_alloc_prepare_to_free(struct binder_alloc *alloc,
 						   unsigned long user_ptr)
 {
-	struct binder_buffer *buffer;
-
-	mutex_lock(&alloc->mutex);
-	buffer = binder_alloc_prepare_to_free_locked(alloc, user_ptr);
-	mutex_unlock(&alloc->mutex);
-	return buffer;
+	guard(mutex)(&alloc->mutex);
+	return binder_alloc_prepare_to_free_locked(alloc, user_ptr);
 }
 
 static inline void
@@ -1043,7 +1039,7 @@ void binder_alloc_print_allocated(struct seq_file *m,
 	struct binder_buffer *buffer;
 	struct rb_node *n;
 
-	mutex_lock(&alloc->mutex);
+	guard(mutex)(&alloc->mutex);
 	for (n = rb_first(&alloc->allocated_buffers); n; n = rb_next(n)) {
 		buffer = rb_entry(n, struct binder_buffer, rb_node);
 		seq_printf(m, "  buffer %d: %lx size %zd:%zd:%zd %s\n",
@@ -1053,7 +1049,6 @@ void binder_alloc_print_allocated(struct seq_file *m,
 			   buffer->extra_buffers_size,
 			   buffer->transaction ? "active" : "delivered");
 	}
-	mutex_unlock(&alloc->mutex);
 }
 
 /**
@@ -1102,10 +1097,9 @@ int binder_alloc_get_allocated_count(struct binder_alloc *alloc)
 	struct rb_node *n;
 	int count = 0;
 
-	mutex_lock(&alloc->mutex);
+	guard(mutex)(&alloc->mutex);
 	for (n = rb_first(&alloc->allocated_buffers); n != NULL; n = rb_next(n))
 		count++;
-	mutex_unlock(&alloc->mutex);
 	return count;
 }
 
diff --git a/drivers/android/binder_alloc.h b/drivers/android/binder_alloc.h
index feecd7414241..a9d5f3169e12 100644
--- a/drivers/android/binder_alloc.h
+++ b/drivers/android/binder_alloc.h
@@ -160,12 +160,8 @@ void binder_alloc_print_pages(struct seq_file *m,
 static inline size_t
 binder_alloc_get_free_async_space(struct binder_alloc *alloc)
 {
-	size_t free_async_space;
-
-	mutex_lock(&alloc->mutex);
-	free_async_space = alloc->free_async_space;
-	mutex_unlock(&alloc->mutex);
-	return free_async_space;
+	guard(mutex)(&alloc->mutex);
+	return alloc->free_async_space;
 }
 
 unsigned long
-- 
2.50.0

Re: [PATCH 1/2] binder: use kstrdup() in binderfs_binder_device_create()

[Tiffany Yang]

Dmitry Antipov <dmantipov@yandex.ru> writes:

> In 'binderfs_binder_device_create()', use 'kstrdup()' to copy the
> newly created device's name, thus making the former a bit simpler.
>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>  drivers/android/binderfs.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> index 024275dbfdd8..4f827152d18e 100644
> --- a/drivers/android/binderfs.c
> +++ b/drivers/android/binderfs.c
> @@ -117,7 +117,6 @@ static int binderfs_binder_device_create(struct inode *ref_inode,
>  	struct dentry *dentry, *root;
>  	struct binder_device *device;
>  	char *name = NULL;
> -	size_t name_len;
>  	struct inode *inode = NULL;
>  	struct super_block *sb = ref_inode->i_sb;
>  	struct binderfs_info *info = sb->s_fs_info;
> @@ -161,9 +160,7 @@ static int binderfs_binder_device_create(struct inode *ref_inode,
>  	inode->i_gid = info->root_gid;
>  
>  	req->name[BINDERFS_MAX_NAME] = '\0'; /* NUL-terminate */
> -	name_len = strlen(req->name);
> -	/* Make sure to include terminating NUL byte */
> -	name = kmemdup(req->name, name_len + 1, GFP_KERNEL);
> +	name = kstrdup(req->name, GFP_KERNEL);
>  	if (!name)
>  		goto err;

Reviewed-by: Tiffany Y. Yang <ynaffit@google.com>

-- 
Tiffany Y. Yang

Re: [PATCH 1/2] binder: use kstrdup() in binderfs_binder_device_create()

[Carlos Llamas]

On Thu, Jun 26, 2025 at 10:30:53AM +0300, Dmitry Antipov wrote:
> In 'binderfs_binder_device_create()', use 'kstrdup()' to copy the
> newly created device's name, thus making the former a bit simpler.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
>  drivers/android/binderfs.c | 5 +----
>  1 file changed, 1 insertion(+), 4 deletions(-)
> 
> diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> index 024275dbfdd8..4f827152d18e 100644
> --- a/drivers/android/binderfs.c
> +++ b/drivers/android/binderfs.c
> @@ -117,7 +117,6 @@ static int binderfs_binder_device_create(struct inode *ref_inode,
>  	struct dentry *dentry, *root;
>  	struct binder_device *device;
>  	char *name = NULL;
> -	size_t name_len;
>  	struct inode *inode = NULL;
>  	struct super_block *sb = ref_inode->i_sb;
>  	struct binderfs_info *info = sb->s_fs_info;
> @@ -161,9 +160,7 @@ static int binderfs_binder_device_create(struct inode *ref_inode,
>  	inode->i_gid = info->root_gid;
>  
>  	req->name[BINDERFS_MAX_NAME] = '\0'; /* NUL-terminate */
> -	name_len = strlen(req->name);
> -	/* Make sure to include terminating NUL byte */
> -	name = kmemdup(req->name, name_len + 1, GFP_KERNEL);
> +	name = kstrdup(req->name, GFP_KERNEL);
>  	if (!name)
>  		goto err;
>  
> -- 
> 2.50.0
> 

Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

Thanks Dimitry,

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

Re: [PATCH 2/2] binder: use guards for plain mutex- and spinlock-protected sections

[Carlos Llamas]

On Thu, Jun 26, 2025 at 10:30:54AM +0300, Dmitry Antipov wrote:
> Use 'guard(mutex)' and 'guard(spinlock)' for plain (i.e. non-scoped)
> mutex- and spinlock-protected sections, respectively, thus making
> locking a bit simpler. Briefly tested with 'stress-ng --binderfs'.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

This looks correct. Thanks!

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

Re: [PATCH 1/2] binder: use kstrdup() in binderfs_binder_device_create()

[Alice Ryhl]

On Thu, Jun 26, 2025 at 10:30:53AM +0300, Dmitry Antipov wrote:
> In 'binderfs_binder_device_create()', use 'kstrdup()' to copy the
> newly created device's name, thus making the former a bit simpler.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>

Re: [PATCH 2/2] binder: use guards for plain mutex- and spinlock-protected sections

[Alice Ryhl]

On Thu, Jun 26, 2025 at 10:30:54AM +0300, Dmitry Antipov wrote:
> Use 'guard(mutex)' and 'guard(spinlock)' for plain (i.e. non-scoped)
> mutex- and spinlock-protected sections, respectively, thus making
> locking a bit simpler. Briefly tested with 'stress-ng --binderfs'.
> 
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>

Reviewed-by: Alice Ryhl <aliceryhl@google.com>