* [PATCH v2 0/2] Add default binderfs devices @ 2019-08-06 18:40 Hridya Valsaraju 2019-08-06 18:40 ` [PATCH v2 1/2] binder: Add default binder devices through binderfs when configured Hridya Valsaraju 2019-08-06 18:40 ` [PATCH v2 2/2] binder: Validate the default binderfs device names Hridya Valsaraju 0 siblings, 2 replies; 7+ messages in thread From: Hridya Valsaraju @ 2019-08-06 18:40 UTC (permalink / raw) To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Martijn Coenen, Joel Fernandes, Christian Brauner, devel, linux-kernel Cc: kernel-team, Hridya Valsaraju Binderfs was created to help provide private binder devices to containers in their own IPC namespace. Currently, every time a new binderfs instance is mounted, its private binder devices need to be created via IOCTL calls. This patch series eliminates the effort for creating the default binder devices for each binderfs instance by creating them automatically. Hridya Valsaraju (2): binder: Add default binder devices through binderfs when configured binder: Validate the default binderfs device names. drivers/android/binder.c | 5 +++-- drivers/android/binder_internal.h | 2 ++ drivers/android/binderfs.c | 37 ++++++++++++++++++++++++++++--- 3 files changed, 39 insertions(+), 5 deletions(-) -- 2.22.0.770.g0f2c4a37fd-goog ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 1/2] binder: Add default binder devices through binderfs when configured 2019-08-06 18:40 [PATCH v2 0/2] Add default binderfs devices Hridya Valsaraju @ 2019-08-06 18:40 ` Hridya Valsaraju 2019-08-07 11:02 ` Dan Carpenter 2019-08-06 18:40 ` [PATCH v2 2/2] binder: Validate the default binderfs device names Hridya Valsaraju 1 sibling, 1 reply; 7+ messages in thread From: Hridya Valsaraju @ 2019-08-06 18:40 UTC (permalink / raw) To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Martijn Coenen, Joel Fernandes, Christian Brauner, devel, linux-kernel Cc: kernel-team, Hridya Valsaraju, Christian Brauner Currently, since each binderfs instance needs its own private binder devices, every time a binderfs instance is mounted, all the default binder devices need to be created via the BINDER_CTL_ADD IOCTL. This patch aims to add a solution to automatically create the default binder devices for each binderfs instance that gets mounted. To achieve this goal, when CONFIG_ANDROID_BINDERFS is set, the default binder devices specified by CONFIG_ANDROID_BINDER_DEVICES are created in each binderfs instance instead of global devices being created by the binder driver. Co-developed-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Hridya Valsaraju <hridya@google.com> --- Changes in v2: - Updated commit message as per Greg Kroah-Hartman. - Removed new module parameter creation as per Greg Kroah-Hartman/Christian Brauner. - Refactored device name length check into a new patch as per Greg Kroah-Hartman. drivers/android/binder.c | 5 +++-- drivers/android/binder_internal.h | 2 ++ drivers/android/binderfs.c | 25 ++++++++++++++++++++++--- 3 files changed, 27 insertions(+), 5 deletions(-) diff --git a/drivers/android/binder.c b/drivers/android/binder.c index 466b6a7f8ab7..ca6b21a53321 100644 --- a/drivers/android/binder.c +++ b/drivers/android/binder.c @@ -123,7 +123,7 @@ static uint32_t binder_debug_mask = BINDER_DEBUG_USER_ERROR | BINDER_DEBUG_FAILED_TRANSACTION | BINDER_DEBUG_DEAD_TRANSACTION; module_param_named(debug_mask, binder_debug_mask, uint, 0644); -static char *binder_devices_param = CONFIG_ANDROID_BINDER_DEVICES; +char *binder_devices_param = CONFIG_ANDROID_BINDER_DEVICES; module_param_named(devices, binder_devices_param, charp, 0444); static DECLARE_WAIT_QUEUE_HEAD(binder_user_error_wait); @@ -6279,7 +6279,8 @@ static int __init binder_init(void) &transaction_log_fops); } - if (strcmp(binder_devices_param, "") != 0) { + if (!IS_ENABLED(CONFIG_ANDROID_BINDERFS) && + strcmp(binder_devices_param, "") != 0) { /* * Copy the module_parameter string, because we don't want to * tokenize it in-place. diff --git a/drivers/android/binder_internal.h b/drivers/android/binder_internal.h index 045b3e42d98b..fe8c745dc8e0 100644 --- a/drivers/android/binder_internal.h +++ b/drivers/android/binder_internal.h @@ -37,6 +37,8 @@ struct binder_device { extern const struct file_operations binder_fops; +extern char *binder_devices_param; + #ifdef CONFIG_ANDROID_BINDERFS extern bool is_binderfs_device(const struct inode *inode); #else diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c index e773f45d19d9..886b4e0f482f 100644 --- a/drivers/android/binderfs.c +++ b/drivers/android/binderfs.c @@ -186,8 +186,7 @@ static int binderfs_binder_device_create(struct inode *ref_inode, req->major = MAJOR(binderfs_dev); req->minor = minor; - ret = copy_to_user(userp, req, sizeof(*req)); - if (ret) { + if (userp && copy_to_user(userp, req, sizeof(*req))) { ret = -EFAULT; goto err; } @@ -467,6 +466,9 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent) int ret; struct binderfs_info *info; struct inode *inode = NULL; + struct binderfs_device device_info = { 0 }; + const char *name; + size_t len; sb->s_blocksize = PAGE_SIZE; sb->s_blocksize_bits = PAGE_SHIFT; @@ -521,7 +523,24 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent) if (!sb->s_root) return -ENOMEM; - return binderfs_binder_ctl_create(sb); + ret = binderfs_binder_ctl_create(sb); + if (ret) + return ret; + + name = binder_devices_param; + for (len = strcspn(name, ","); len > 0; len = strcspn(name, ",")) { + strscpy(device_info.name, name, len + 1); + ret = binderfs_binder_device_create(inode, NULL, &device_info); + if (ret) + return ret; + name += len; + if (*name == ',') + name++; + + } + + return 0; + } static struct dentry *binderfs_mount(struct file_system_type *fs_type, -- 2.22.0.770.g0f2c4a37fd-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] binder: Add default binder devices through binderfs when configured 2019-08-06 18:40 ` [PATCH v2 1/2] binder: Add default binder devices through binderfs when configured Hridya Valsaraju @ 2019-08-07 11:02 ` Dan Carpenter 2019-08-07 16:51 ` Hridya Valsaraju 2019-08-07 17:50 ` Christian Brauner 0 siblings, 2 replies; 7+ messages in thread From: Dan Carpenter @ 2019-08-07 11:02 UTC (permalink / raw) To: Hridya Valsaraju Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Martijn Coenen, Joel Fernandes, Christian Brauner, devel, linux-kernel, Christian Brauner, kernel-team On Tue, Aug 06, 2019 at 11:40:05AM -0700, Hridya Valsaraju wrote: > @@ -467,6 +466,9 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent) > int ret; > struct binderfs_info *info; > struct inode *inode = NULL; > + struct binderfs_device device_info = { 0 }; > + const char *name; > + size_t len; > > sb->s_blocksize = PAGE_SIZE; > sb->s_blocksize_bits = PAGE_SHIFT; > @@ -521,7 +523,24 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent) > if (!sb->s_root) > return -ENOMEM; > > - return binderfs_binder_ctl_create(sb); > + ret = binderfs_binder_ctl_create(sb); > + if (ret) > + return ret; > + > + name = binder_devices_param; > + for (len = strcspn(name, ","); len > 0; len = strcspn(name, ",")) { > + strscpy(device_info.name, name, len + 1); > + ret = binderfs_binder_device_create(inode, NULL, &device_info); > + if (ret) > + return ret; We should probably clean up before returning... The error handling code would probably be tricky to write though and it's not super common. > + name += len; > + if (*name == ',') > + name++; > + > + } > + > + return 0; > + Remove this extra blank line. > } regards, dan carpenter ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] binder: Add default binder devices through binderfs when configured 2019-08-07 11:02 ` Dan Carpenter @ 2019-08-07 16:51 ` Hridya Valsaraju 2019-08-09 9:25 ` Dan Carpenter 2019-08-07 17:50 ` Christian Brauner 1 sibling, 1 reply; 7+ messages in thread From: Hridya Valsaraju @ 2019-08-07 16:51 UTC (permalink / raw) To: Dan Carpenter Cc: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Martijn Coenen, Joel Fernandes, Christian Brauner, devel, linux-kernel, Christian Brauner, kernel-team On Wed, Aug 7, 2019 at 4:02 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > On Tue, Aug 06, 2019 at 11:40:05AM -0700, Hridya Valsaraju wrote: > > @@ -467,6 +466,9 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent) > > int ret; > > struct binderfs_info *info; > > struct inode *inode = NULL; > > + struct binderfs_device device_info = { 0 }; > > + const char *name; > > + size_t len; > > > > sb->s_blocksize = PAGE_SIZE; > > sb->s_blocksize_bits = PAGE_SHIFT; > > @@ -521,7 +523,24 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent) > > if (!sb->s_root) > > return -ENOMEM; > > > > - return binderfs_binder_ctl_create(sb); > > + ret = binderfs_binder_ctl_create(sb); > > + if (ret) > > + return ret; > > + > > + name = binder_devices_param; > > + for (len = strcspn(name, ","); len > 0; len = strcspn(name, ",")) { > > + strscpy(device_info.name, name, len + 1); > > + ret = binderfs_binder_device_create(inode, NULL, &device_info); > > + if (ret) > > + return ret; > > We should probably clean up before returning... The error handling code > would probably be tricky to write though and it's not super common. Thank you for taking a look Dan. Did you mean cleaning up the default devices that were already created? They will actually be cleaned up by binderfs_evict_inode() during the super block's cleanup since the mount operation will fail due to an error here. > > > + name += len; > > + if (*name == ',') > > + name++; > > + > > + } > > + > > + return 0; > > + > > Remove this extra blank line. Will do in v3, thanks for catching this Dan! > > > } > > regards, > dan carpenter > > -- > To unsubscribe from this group and stop receiving emails from it, send an email to kernel-team+unsubscribe@android.com. > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] binder: Add default binder devices through binderfs when configured 2019-08-07 16:51 ` Hridya Valsaraju @ 2019-08-09 9:25 ` Dan Carpenter 0 siblings, 0 replies; 7+ messages in thread From: Dan Carpenter @ 2019-08-09 9:25 UTC (permalink / raw) To: Hridya Valsaraju Cc: devel, kernel-team, Todd Kjos, Greg Kroah-Hartman, linux-kernel, Arve Hjønnevåg, Joel Fernandes, Christian Brauner, Martijn Coenen, Christian Brauner On Wed, Aug 07, 2019 at 09:51:46AM -0700, Hridya Valsaraju wrote: > On Wed, Aug 7, 2019 at 4:02 AM Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > > On Tue, Aug 06, 2019 at 11:40:05AM -0700, Hridya Valsaraju wrote: > > > @@ -467,6 +466,9 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent) > > > int ret; > > > struct binderfs_info *info; > > > struct inode *inode = NULL; > > > + struct binderfs_device device_info = { 0 }; > > > + const char *name; > > > + size_t len; > > > > > > sb->s_blocksize = PAGE_SIZE; > > > sb->s_blocksize_bits = PAGE_SHIFT; > > > @@ -521,7 +523,24 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent) > > > if (!sb->s_root) > > > return -ENOMEM; > > > > > > - return binderfs_binder_ctl_create(sb); > > > + ret = binderfs_binder_ctl_create(sb); > > > + if (ret) > > > + return ret; > > > + > > > + name = binder_devices_param; > > > + for (len = strcspn(name, ","); len > 0; len = strcspn(name, ",")) { > > > + strscpy(device_info.name, name, len + 1); > > > + ret = binderfs_binder_device_create(inode, NULL, &device_info); > > > + if (ret) > > > + return ret; > > > > We should probably clean up before returning... The error handling code > > would probably be tricky to write though and it's not super common. > > Thank you for taking a look Dan. Did you mean cleaning up the default > devices that were already created? They will actually be cleaned up by > binderfs_evict_inode() during the super block's cleanup since the > mount operation will fail due to an error here. Yeah. I meant the binderfs_binder_device_create() from previous iterations through this loop. Good to know that it's handled. Thanks for taking the time to look at this. regards, dan carpenter ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] binder: Add default binder devices through binderfs when configured 2019-08-07 11:02 ` Dan Carpenter 2019-08-07 16:51 ` Hridya Valsaraju @ 2019-08-07 17:50 ` Christian Brauner 1 sibling, 0 replies; 7+ messages in thread From: Christian Brauner @ 2019-08-07 17:50 UTC (permalink / raw) To: Dan Carpenter Cc: Hridya Valsaraju, Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Martijn Coenen, Joel Fernandes, Christian Brauner, devel, linux-kernel, Christian Brauner, kernel-team On Wed, Aug 07, 2019 at 02:02:05PM +0300, Dan Carpenter wrote: > On Tue, Aug 06, 2019 at 11:40:05AM -0700, Hridya Valsaraju wrote: > > @@ -467,6 +466,9 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent) > > int ret; > > struct binderfs_info *info; > > struct inode *inode = NULL; > > + struct binderfs_device device_info = { 0 }; > > + const char *name; > > + size_t len; > > > > sb->s_blocksize = PAGE_SIZE; > > sb->s_blocksize_bits = PAGE_SHIFT; > > @@ -521,7 +523,24 @@ static int binderfs_fill_super(struct super_block *sb, void *data, int silent) > > if (!sb->s_root) > > return -ENOMEM; > > > > - return binderfs_binder_ctl_create(sb); > > + ret = binderfs_binder_ctl_create(sb); > > + if (ret) > > + return ret; > > + > > + name = binder_devices_param; > > + for (len = strcspn(name, ","); len > 0; len = strcspn(name, ",")) { > > + strscpy(device_info.name, name, len + 1); > > + ret = binderfs_binder_device_create(inode, NULL, &device_info); > > + if (ret) > > + return ret; > > We should probably clean up before returning... The error handling code > would probably be tricky to write though and it's not super common. struct dentry *mount_nodev(struct file_system_type *fs_type, int flags, void *data, int (*fill_super)(struct super_block *, void *, int)) { <snip> error = fill_super(s, data, flags & SB_SILENT ? 1 : 0); if (error) { deactivate_locked_super(s); return ERR_PTR(error); } <snip> } deactivate_locked_super() will call fs->kill_sb(s) which calls binderfs_kill_super() which calls kill_litter_super() the latter will destory any remaining dentries and then calls generic_shutdown_super() which calls evict_inodes() which calls evict(inode) which calls the binderfs specific binderfs_evict_inode() and get rid of the rest. So manually cleaning up is not needed, imho. Christian ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] binder: Validate the default binderfs device names. 2019-08-06 18:40 [PATCH v2 0/2] Add default binderfs devices Hridya Valsaraju 2019-08-06 18:40 ` [PATCH v2 1/2] binder: Add default binder devices through binderfs when configured Hridya Valsaraju @ 2019-08-06 18:40 ` Hridya Valsaraju 1 sibling, 0 replies; 7+ messages in thread From: Hridya Valsaraju @ 2019-08-06 18:40 UTC (permalink / raw) To: Greg Kroah-Hartman, Arve Hjønnevåg, Todd Kjos, Martijn Coenen, Joel Fernandes, Christian Brauner, devel, linux-kernel Cc: kernel-team, Hridya Valsaraju, Christian Brauner Length of a binderfs device name cannot exceed BINDERFS_MAX_NAME. This patch adds a check in binderfs_init() to ensure the same for the default binder devices that will be created in every binderfs instance. Co-developed-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com> Signed-off-by: Hridya Valsaraju <hridya@google.com> --- drivers/android/binderfs.c | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c index 886b4e0f482f..52c8bd361906 100644 --- a/drivers/android/binderfs.c +++ b/drivers/android/binderfs.c @@ -572,6 +572,18 @@ static struct file_system_type binder_fs_type = { int __init init_binderfs(void) { int ret; + const char *name; + size_t len; + + /* Verify that the default binderfs device names are valid. */ + name = binder_devices_param; + for (len = strcspn(name, ","); len > 0; len = strcspn(name, ",")) { + if (len > BINDERFS_MAX_NAME) + return -E2BIG; + name += len; + if (*name == ',') + name++; + } /* Allocate new major number for binderfs. */ ret = alloc_chrdev_region(&binderfs_dev, 0, BINDERFS_MAX_MINOR, -- 2.22.0.770.g0f2c4a37fd-goog ^ permalink raw reply related [flat|nested] 7+ messages in thread
end of thread, other threads:[~2019-08-09 9:25 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2019-08-06 18:40 [PATCH v2 0/2] Add default binderfs devices Hridya Valsaraju 2019-08-06 18:40 ` [PATCH v2 1/2] binder: Add default binder devices through binderfs when configured Hridya Valsaraju 2019-08-07 11:02 ` Dan Carpenter 2019-08-07 16:51 ` Hridya Valsaraju 2019-08-09 9:25 ` Dan Carpenter 2019-08-07 17:50 ` Christian Brauner 2019-08-06 18:40 ` [PATCH v2 2/2] binder: Validate the default binderfs device names Hridya Valsaraju
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).