From: Al Viro <viro@zeniv.linux.org.uk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Josh Triplett <josh@joshtriplett.org>,
Kees Cook <kees@kernel.org>, Kees Cook <keescook@chromium.org>,
linux-kernel@vger.kernel.org,
Alexey Dobriyan <adobriyan@gmail.com>
Subject: Re: [GIT PULL] execve updates for v6.8-rc1
Date: Thu, 11 Jan 2024 10:05:01 +0000 [thread overview]
Message-ID: <20240111100501.GU1674809@ZenIV> (raw)
In-Reply-To: <20240111094711.GT1674809@ZenIV>
On Thu, Jan 11, 2024 at 09:47:11AM +0000, Al Viro wrote:
> Doable, but really not pretty, especially since we'd need to massage
> the caller as well... Less painful variant is
> if (error == -ECHILD && (flags & LOOKUP_RCU))
> return ERR_PTR(-ECHILD); // keep file for non-rcu pass
> *fp = NULL;
> fput(file);
> ...
> on the way out; that won't help with -ESTALE side of things, but if we
> hit *that*, struct file allocation overhead is really noise.
Something like (completely untested) delta below, perhaps?
diff --git a/fs/namei.c b/fs/namei.c
index 5c318d657503..de770be9bb16 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3765,15 +3765,17 @@ static int do_o_path(struct nameidata *nd, unsigned flags, struct file *file)
return error;
}
-static struct file *path_openat(struct nameidata *nd,
+static int path_openat(struct nameidata *nd, struct file **fp,
const struct open_flags *op, unsigned flags)
{
- struct file *file;
+ struct file *file = *fp;
int error;
- file = alloc_empty_file(op->open_flag, current_cred());
- if (IS_ERR(file))
- return file;
+ if (!file) {
+ file = alloc_empty_file(op->open_flag, current_cred());
+ if (IS_ERR(file))
+ return PTR_ERR(file);
+ }
if (unlikely(file->f_flags & __O_TMPFILE)) {
error = do_tmpfile(nd, flags, op, file);
@@ -3789,11 +3791,17 @@ static struct file *path_openat(struct nameidata *nd,
terminate_walk(nd);
}
if (likely(!error)) {
- if (likely(file->f_mode & FMODE_OPENED))
- return file;
+ if (likely(file->f_mode & FMODE_OPENED)) {
+ *fp = file;
+ return 0;
+ }
WARN_ON(1);
error = -EINVAL;
}
+ if (error == -ECHILD && (flags & LOOKUP_RCU)) {
+ *fp = file; // reuse on the next pass
+ return -ECHILD;
+ }
fput(file);
if (error == -EOPENSTALE) {
if (flags & LOOKUP_RCU)
@@ -3801,7 +3809,7 @@ static struct file *path_openat(struct nameidata *nd,
else
error = -ESTALE;
}
- return ERR_PTR(error);
+ return error;
}
struct file *do_filp_open(int dfd, struct filename *pathname,
@@ -3809,25 +3817,27 @@ struct file *do_filp_open(int dfd, struct filename *pathname,
{
struct nameidata nd;
int flags = op->lookup_flags;
- struct file *filp;
+ struct file *file = NULL;
+ int err;
set_nameidata(&nd, dfd, pathname, NULL);
- filp = path_openat(&nd, op, flags | LOOKUP_RCU);
- if (unlikely(filp == ERR_PTR(-ECHILD)))
- filp = path_openat(&nd, op, flags);
- if (unlikely(filp == ERR_PTR(-ESTALE)))
- filp = path_openat(&nd, op, flags | LOOKUP_REVAL);
+ err = path_openat(&nd, &file, op, flags | LOOKUP_RCU);
+ if (unlikely(err == -ECHILD))
+ err = path_openat(&nd, &file, op, flags);
+ if (unlikely(err == -ESTALE))
+ err = path_openat(&nd, &file, op, flags | LOOKUP_REVAL);
restore_nameidata();
- return filp;
+ return unlikely(err) ? ERR_PTR(err) : file;
}
struct file *do_file_open_root(const struct path *root,
const char *name, const struct open_flags *op)
{
struct nameidata nd;
- struct file *file;
+ struct file *file = NULL;
struct filename *filename;
int flags = op->lookup_flags;
+ int err;
if (d_is_symlink(root->dentry) && op->intent & LOOKUP_OPEN)
return ERR_PTR(-ELOOP);
@@ -3837,14 +3847,14 @@ struct file *do_file_open_root(const struct path *root,
return ERR_CAST(filename);
set_nameidata(&nd, -1, filename, root);
- file = path_openat(&nd, op, flags | LOOKUP_RCU);
- if (unlikely(file == ERR_PTR(-ECHILD)))
- file = path_openat(&nd, op, flags);
- if (unlikely(file == ERR_PTR(-ESTALE)))
- file = path_openat(&nd, op, flags | LOOKUP_REVAL);
+ err = path_openat(&nd, &file, op, flags | LOOKUP_RCU);
+ if (unlikely(err == -ECHILD))
+ err = path_openat(&nd, &file, op, flags);
+ if (unlikely(err == -ESTALE))
+ err = path_openat(&nd, &file, op, flags | LOOKUP_REVAL);
restore_nameidata();
putname(filename);
- return file;
+ return unlikely(err) ? ERR_PTR(err) : file;
}
static struct dentry *filename_create(int dfd, struct filename *name,
next prev parent reply other threads:[~2024-01-11 10:05 UTC|newest]
Thread overview: 19+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-01-08 18:35 [GIT PULL] execve updates for v6.8-rc1 Kees Cook
2024-01-09 0:19 ` Linus Torvalds
2024-01-09 0:30 ` Linus Torvalds
2024-01-09 0:46 ` Linus Torvalds
2024-01-09 1:48 ` Kees Cook
2024-01-09 1:53 ` Linus Torvalds
2024-01-09 3:28 ` Linus Torvalds
2024-01-09 18:57 ` Josh Triplett
2024-01-09 23:40 ` Linus Torvalds
2024-01-10 2:21 ` Josh Triplett
2024-01-10 3:54 ` Linus Torvalds
2024-01-11 9:47 ` Al Viro
2024-01-11 10:05 ` Al Viro [this message]
2024-01-11 17:42 ` Linus Torvalds
2024-01-20 22:18 ` Linus Torvalds
2024-01-21 8:05 ` Kees Cook
2024-01-11 17:37 ` Linus Torvalds
2024-01-10 19:24 ` Kees Cook
2024-01-10 20:12 ` 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=20240111100501.GU1674809@ZenIV \
--to=viro@zeniv.linux.org.uk \
--cc=adobriyan@gmail.com \
--cc=josh@joshtriplett.org \
--cc=kees@kernel.org \
--cc=keescook@chromium.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).