binderfs: rework binderfs_binder_device_create()

- 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@zeniv.linux.org.uk>
Signed-off-by: Christian Brauner <christian@brauner.io>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

diff --git a/drivers/android/binderfs.c b/drivers/android/binderfs.c
index 89a2ee1a02f6513ea2fd52f3ab30d4f56bb25bd6..1e077498a5070ba5b71c4b17828bca20717e2d4a 100644 (file)
--- 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;