[PATCH v1 4/7] binderfs: rework binderfs_binder_device_create()

[Christian Brauner <christian@xxxxxxxxxx>]

- switch from d_alloc_name() + d_lookup() to lookup_one_len():
  Instead of using d_alloc_name() and then doing a d_lookup() with the
  allocated dentry to find whether a device with the name we're trying to
  create already exists switch to using lookup_one_len().  The latter will
  either return the existing dentry or a new one.

- switch from kmalloc() + strscpy() to kmemdup():
  Use a more idiomatic way to copy the name for the new dentry that
  userspace gave us.

Suggested-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Signed-off-by: Christian Brauner <christian@xxxxxxxxxx>
---
/* Changelog */

v1:
- patch introduced
---
 drivers/android/binderfs.c | 39 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 20 deletions(-)

diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index 89a2ee1a02f6..1e077498a507 100644
--- a/drivers/android/binderfs.c
+++ b/drivers/android/binderfs.c
@@ -11,6 +11,7 @@
 #include <linux/kdev_t.h>
 #include <linux/kernel.h>
 #include <linux/list.h>
+#include <linux/namei.h>
 #include <linux/magic.h>
 #include <linux/major.h>
 #include <linux/miscdevice.h>
@@ -106,7 +107,7 @@ bool is_binderfs_device(const struct inode *inode)
  * @userp:     buffer to copy information about new device for userspace to
  * @req:       struct binderfs_device as copied from userspace
  *
- * This function allocated a new binder_device and reserves a new minor
+ * This function allocates a new binder_device and reserves a new minor
  * number for it.
  * Minor numbers are limited and tracked globally in binderfs_minors. The
  * function will stash a struct binder_device for the specific binder
@@ -122,10 +123,10 @@ static int binderfs_binder_device_create(struct inode *ref_inode,
 					 struct binderfs_device *req)
 {
 	int minor, ret;
-	struct dentry *dentry, *dup, *root;
+	struct dentry *dentry, *root;
 	struct binder_device *device;
-	size_t name_len = BINDERFS_MAX_NAME + 1;
 	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;
@@ -168,12 +169,13 @@ static int binderfs_binder_device_create(struct inode *ref_inode,
 	inode->i_uid = info->root_uid;
 	inode->i_gid = info->root_gid;
 
-	name = kmalloc(name_len, GFP_KERNEL);
+	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);
 	if (!name)
 		goto err;
 
-	strscpy(name, req->name, name_len);
-
 	device->binderfs_inode = inode;
 	device->context.binder_context_mgr_uid = INVALID_UID;
 	device->context.name = name;
@@ -192,24 +194,21 @@ static int binderfs_binder_device_create(struct inode *ref_inode,
 
 	root = sb->s_root;
 	inode_lock(d_inode(root));
-	dentry = d_alloc_name(root, name);
-	if (!dentry) {
+
+	/* look it up */
+	dentry = lookup_one_len(name, root, name_len);
+	if (IS_ERR(dentry)) {
 		inode_unlock(d_inode(root));
-		ret = -ENOMEM;
+		ret = PTR_ERR(dentry);
 		goto err;
 	}
 
-	/* Verify that the name userspace gave us is not already in use. */
-	dup = d_lookup(root, &dentry->d_name);
-	if (dup) {
-		if (d_really_is_positive(dup)) {
-			dput(dup);
-			dput(dentry);
-			inode_unlock(d_inode(root));
-			ret = -EEXIST;
-			goto err;
-		}
-		dput(dup);
+	if (d_really_is_positive(dentry)) {
+		/* already exists */
+		dput(dentry);
+		inode_unlock(d_inode(root));
+		ret = -EEXIST;
+		goto err;
 	}
 
 	inode->i_private = device;
-- 
2.19.1

_______________________________________________
devel mailing list
devel@xxxxxxxxxxxxxxxxxxxxxx
http://driverdev.linuxdriverproject.org/mailman/listinfo/driverdev-devel