[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20240111100501.GU1674809@ZenIV>
Date: Thu, 11 Jan 2024 10:05:01 +0000
From: Al Viro <viro@...iv.linux.org.uk>
To: Linus Torvalds <torvalds@...ux-foundation.org>
Cc: Josh Triplett <josh@...htriplett.org>, Kees Cook <kees@...nel.org>,
Kees Cook <keescook@...omium.org>, linux-kernel@...r.kernel.org,
Alexey Dobriyan <adobriyan@...il.com>
Subject: Re: [GIT PULL] execve updates for v6.8-rc1
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,
Powered by blists - more mailing lists