linux-kernel - Re: [PATCH net-next v10 1/2] binderfs: add new binder devices to binder

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: <Z2Bq9CjXO6FXIFjl@google.com>
Date: Mon, 16 Dec 2024 18:01:24 +0000
From: Carlos Llamas <cmllamas@...gle.com>
To: Li Li <dualli@...omium.org>
Cc: dualli@...gle.com, corbet@....net, davem@...emloft.net,
	edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
	donald.hunter@...il.com, gregkh@...uxfoundation.org,
	arve@...roid.com, tkjos@...roid.com, maco@...roid.com,
	joel@...lfernandes.org, brauner@...nel.org, surenb@...gle.com,
	arnd@...db.de, masahiroy@...nel.org, bagasdotme@...il.com,
	horms@...nel.org, linux-kernel@...r.kernel.org,
	linux-doc@...r.kernel.org, netdev@...r.kernel.org,
	hridya@...gle.com, smoreland@...gle.com, kernel-team@...roid.com
Subject: Re: [PATCH net-next v10 1/2] binderfs: add new binder devices to
 binder_devices

On Thu, Dec 12, 2024 at 02:41:13PM -0800, Li Li wrote:
> From: Li Li <dualli@...gle.com>
> 
> When binderfs is not enabled, the binder driver parses the kernel
> config to create all binder devices. All of the new binder devices
> are stored in the list binder_devices.
> 
> When binderfs is enabled, the binder driver creates new binder devices
> dynamically when userspace applications call BINDER_CTL_ADD ioctl. But
> the devices created in this way are not stored in the same list.
> 
> This patch fixes that.
> 
> Signed-off-by: Li Li <dualli@...gle.com>
> ---
>  drivers/android/binder.c          | 5 +++++
>  drivers/android/binder_internal.h | 8 ++++++++
>  drivers/android/binderfs.c        | 2 ++
>  3 files changed, 15 insertions(+)
> 
> diff --git a/drivers/android/binder.c b/drivers/android/binder.c
> index ef353ca13c35..0a16acd29653 100644
> --- a/drivers/android/binder.c
> +++ b/drivers/android/binder.c
> @@ -6928,6 +6928,11 @@ const struct binder_debugfs_entry binder_debugfs_entries[] = {
>  	{} /* terminator */
>  };
>  
> +void binder_add_device(struct binder_device *device)
> +{
> +	hlist_add_head(&device->hlist, &binder_devices);
> +}
> +
>  static int __init init_binder_device(const char *name)
>  {
>  	int ret;
> diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h

nit: I believe you the following hunk should be in this patch no?

 /**
  * struct binder_device - information about a binder device node
- * @hlist:          list of binder devices (only used for devices requested via
- *                  CONFIG_ANDROID_BINDER_DEVICES)
+ * @hlist:          list of binder devices
  * @miscdev:        information about a binder character device node
  * @context:        binder context information
  * @binderfs_inode: This is the inode of the root dentry of the super block


> index f8d6be682f23..1f21ad3963b1 100644
> --- a/drivers/android/binder_internal.h
> +++ b/drivers/android/binder_internal.h
> @@ -582,4 +582,12 @@ struct binder_object {
>  	};
>  };
>  
> +/**
> + * Add a binder device to binder_devices
> + * @device: the new binder device to add to the global list
> + *
> + * Not reentrant as the list is not protected by any locks
> + */
> +void binder_add_device(struct binder_device *device);
> +
>  #endif /* _LINUX_BINDER_INTERNAL_H */
> diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
> index ad1fa7abc323..bc6bae76ccaf 100644
> --- a/drivers/android/binderfs.c
> +++ b/drivers/android/binderfs.c
> @@ -207,6 +207,8 @@ static int binderfs_binder_device_create(struct inode *ref_inode,
>  	fsnotify_create(root->d_inode, dentry);
>  	inode_unlock(d_inode(root));
>  
> +	binder_add_device(device);
> +
>  	return 0;
>  
>  err:
> -- 
> 2.47.1.613.gc27f4b7a9f-goog
> 


Other than the comment above it LGTM. Feel free to add:

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