From: Al Viro <viro@zeniv.linux.org.uk>
To: Christian Brauner <christian@brauner.io>
Cc: gregkh@linuxfoundation.org, devel@driverdev.osuosl.org,
linux-fsdevel@vger.kernel.org, tkjos@google.com
Subject: Re: [PATCH 0/5] binderfs: debug galore
Date: Fri, 18 Jan 2019 23:26:34 +0000 [thread overview]
Message-ID: <20190118232634.GB2217@ZenIV.linux.org.uk> (raw)
In-Reply-To: <20190118145344.11532-1-christian@brauner.io>
On Fri, Jan 18, 2019 at 03:53:39PM +0100, Christian Brauner wrote:
> Hey everyone,
>
> Al gave me a really helpful review of binderfs and pointed out a range
> of bugs. The most obvious and serious ones have fortunately already been
> taken care of by patches sitting in Greg's char-misc-linus tree. The
> others are hopefully all covered in this patchset.
BTW, binderfs_binder_device_create() looks rather odd - it would be easier
to do this:
inode_lock(d_inode(root));
/* look it up */
dentry = lookup_one_len(name, root, strlen(name));
if (IS_ERR(dentry)) {
/* some kind of error (ENOMEM, permissions) - report */
inode_unlock(d_inode(root));
ret = PTR_ERR(dentry);
goto err;
}
if (d_really_is_positive(dentry)) {
/* already exists */
dput(dentry);
inode_unlock(d_inode(root));
ret = -EEXIST;
goto err;
}
inode->i_private = device;
... and from that point on - as in your variant. Another thing in there:
name = kmalloc(name_len, GFP_KERNEL);
if (!name)
goto err;
strscpy(name, req->name, name_len);
is an odd way to go; more straightforward would be
req->name[BINDERFS_MAX_NAME] = '\0'; /* NUL-terminate */
name = kmemdup(req->name, sizeof(req->name), GEP_KERNEL);
if (!name)
....
next prev parent reply other threads:[~2019-01-18 23:26 UTC|newest]
Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-01-18 14:53 [PATCH 0/5] binderfs: debug galore Christian Brauner
2019-01-18 14:53 ` [PATCH 1/5] binderfs: remove outdated comment Christian Brauner
2019-01-18 14:53 ` [PATCH 2/5] binderfs: prevent renaming the control dentry Christian Brauner
2019-01-18 22:55 ` Al Viro
2019-01-19 15:10 ` Christian Brauner
2019-01-18 14:53 ` [PATCH 3/5] binderfs: rework binderfs_fill_super() Christian Brauner
2019-01-18 23:03 ` Al Viro
2019-01-19 15:12 ` Christian Brauner
2019-01-18 14:53 ` [PATCH 4/5] binderfs: kill_litter_super() before cleanup Christian Brauner
2019-01-18 14:53 ` [PATCH 5/5] binderfs: drop lock in binderfs_binder_ctl_create Christian Brauner
2019-01-18 23:26 ` Al Viro [this message]
2019-01-19 15:55 ` [PATCH 0/5] binderfs: debug galore Christian Brauner
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20190118232634.GB2217@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=christian@brauner.io \
--cc=devel@driverdev.osuosl.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=tkjos@google.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.