From: Al Viro <viro@ZenIV.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: [RFC] corner cases of open() on procfs symlinks
Date: Thu, 6 Jun 2013 02:20:10 +0100 [thread overview]
Message-ID: <20130606012010.GE13110@ZenIV.linux.org.uk> (raw)
I'm not sure whether to treat that as a bug or as a weird misfeature
enshrined in userland ABI:
open("/tmp", O_CREAT, 0) => -EISDIR // LAST_NORM case
open("/", O_CREAT, 0) => -EISDIR // LAST_ROOT
open(".", O_CREAT, 0) => -EISDIR // LAST_DOT
open("..", O_CREAT, 0) => -EISDIR // LAST_DOTDOT
open("/proc/self/cwd", O_CREAT, 0) => success // LAST_BIND
open("/proc/self/cwd/", O_CREAT, 0) => -EISDIR // trailing slashes
At the very least, it's inconsistent. OTOH, it's exposed to userland.
OTTH, SuS says that O_CREAT without O_RDWR or O_WRONLY is undefined, and
either of those two would suffice for -EISDIR in all cases (may_open() takes
care of that).
Another unpleasantness:
open("/proc/self/exe", LOOKUP_DIRECTORY, 0) -> success
That one is clearly a bug. Moreover, getting rid of both of those
bugs actually simplifies the mess in do_last(). I would obviously
like to do that - do_last() is far too convoluted as it is; the only
question is whether we can change the first weirdness... Comments?
FWIW, the simplification of do_last() would look like that:
diff --git a/fs/namei.c b/fs/namei.c
index 85e40d1..617599c 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2690,28 +2690,10 @@ static int do_last(struct nameidata *nd, struct path *path,
nd->flags &= ~LOOKUP_PARENT;
nd->flags |= op->intent;
- switch (nd->last_type) {
- case LAST_DOTDOT:
- case LAST_DOT:
+ if (nd->last_type != LAST_NORM) {
error = handle_dots(nd, nd->last_type);
if (error)
return error;
- /* fallthrough */
- case LAST_ROOT:
- error = complete_walk(nd);
- if (error)
- return error;
- audit_inode(name, nd->path.dentry, 0);
- if (open_flag & O_CREAT) {
- error = -EISDIR;
- goto out;
- }
- goto finish_open;
- case LAST_BIND:
- error = complete_walk(nd);
- if (error)
- return error;
- audit_inode(name, dir, 0);
goto finish_open;
}
@@ -2841,6 +2823,7 @@ finish_lookup:
}
nd->inode = inode;
/* Why this, you ask? _Now_ we might have grown LOOKUP_JUMPED... */
+finish_open:
error = complete_walk(nd);
if (error) {
path_put(&save_parent);
@@ -2853,7 +2836,6 @@ finish_lookup:
if ((nd->flags & LOOKUP_DIRECTORY) && !nd->inode->i_op->lookup)
goto out;
audit_inode(name, nd->path.dentry, 0);
-finish_open:
if (!S_ISREG(nd->inode->i_mode))
will_truncate = false;
next reply other threads:[~2013-06-06 1:20 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-06 1:20 Al Viro [this message]
2013-06-06 1:38 ` [RFC] corner cases of open() on procfs symlinks Linus Torvalds
2013-06-06 2:29 ` Al Viro
2013-06-06 2:40 ` Linus Torvalds
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=20130606012010.GE13110@ZenIV.linux.org.uk \
--to=viro@zeniv.linux.org.uk \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=torvalds@linux-foundation.org \
/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 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).