On 6/10/22 19:28, Stefan Metzmacher wrote:
Am 10.06.22 um 13:06 schrieb Hao Xu:
Hi Stefan,
On 6/9/22 16:57, Stefan Metzmacher wrote:
Hi Jens,
this looks very useful, thanks!
I have an additional feature request to make this even more useful...
IO_OP_ACCEPT allows a fixed descriptor for the listen socket
and then can generate a fixed descriptor for the accepted connection,
correct?
Yes.
It would be extremely useful to also allow that pattern
for IO_OP_OPENAT[2], which currently is not able to get
a fixed descriptor for the dirfd argument (this also applies to
IO_OP_STATX, IO_OP_UNLINK and all others taking a dirfd).
Being able use such a sequence:
OPENTAT2(AT_FDCWD, "directory") => 1 (fixed)
STATX(1 (fixed))
FGETXATTR(1 (fixed)
OPENAT2(1 (fixed), "file") => 2 (fixed)
STATX(2 (fixed))
FGETXATTR(2 (fixed))
CLOSE(1 (fixed)
DUP( 2 (fixed)) => per-process fd for ("file")
I looked briefly how to implement that.
But set_nameidata() takes 'int dfd' to store the value
and it's used later somewhere deep down the stack.
And makes it too complex for me to create patches :-(
Indeed.. dirfd is used in path_init() etc. For me, no idea how to tackle
it for now.We surely can register a fixed descriptor to the process
fdtable but that is against the purpose of fixed file..
I looked at it a bit more and the good thing is that
'struct nameidata' is private to namei.c, which simplifies
getting an overview.
path_init() is the actual only user of nd.dfd
^[1]
and it's used to fill nd.path, either from get_fs_pwd()
for AT_FDCWD and f.file->f_path otherwise.
So might be able to have a function that translated
the fd to struct path early and let the callers pass 'struct path'
instead of 'int dfd'...
Yea, if [1] is true. I wrote something for your reference:
(totally unpolished and untested, just to show an idea)
diff --git a/fs/namei.c b/fs/namei.c
index 1f28d3f463c3..18e11717005c 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -2423,21 +2423,30 @@ static const char *path_init(struct nameidata
*nd, unsigned flags)
nd->inode = nd->path.dentry->d_inode;
}
} else {
- /* Caller must check execute permissions on the starting
path component */
- struct fd f = fdget_raw(nd->dfd);
struct dentry *dentry;
- if (!f.file)
- return ERR_PTR(-EBADF);
+ if (nd->dfd != -1) {
+ /* Caller must check execute permissions on the
starting path component */
+ struct fd f = fdget_raw(nd->dfd);
- dentry = f.file->f_path.dentry;
+ if (!f.file)
+ return ERR_PTR(-EBADF);
- if (*s && unlikely(!d_can_lookup(dentry))) {
- fdput(f);
- return ERR_PTR(-ENOTDIR);
+ dentry = f.file->f_path.dentry;
+
+ if (*s && unlikely(!d_can_lookup(dentry))) {
+ fdput(f);
+ return ERR_PTR(-ENOTDIR);
+ }
+
+ nd->path = f.file->f_path;
+ } else {
+ dentry = nd->path.dentry;
+
+ if (*s && unlikely(!d_can_lookup(dentry)))
+ return ERR_PTR(-ENOTDIR);
}
- nd->path = f.file->f_path;
if (flags & LOOKUP_RCU) {
nd->inode = nd->path.dentry->d_inode;
nd->seq =
read_seqcount_begin(&nd->path.dentry->d_seq);
@@ -2445,7 +2454,9 @@ static const char *path_init(struct nameidata *nd,
unsigned flags)
path_get(&nd->path);
nd->inode = nd->path.dentry->d_inode;
}
- fdput(f);
+ if (dfd != -1)
+ fdput(f);
+
}
/* For scoped-lookups we need to set the root to the dirfd as
well. */
@@ -3686,6 +3697,48 @@ struct file *do_filp_open(int dfd, struct
filename *pathname,
return filp;
}
+static void __set_nameidata2(struct nameidata *p, struct path *path,
+ struct filename *name)
+{
+ struct nameidata *old = current->nameidata;
+ p->stack = p->internal;
+ p->depth = 0;
+ p->dfd = -1;
+ p->name = name;
+ p->path = *path;
+ p->total_link_count = old ? old->total_link_count : 0;
+ p->saved = old;
+ current->nameidata = p;
+}
+
+static inline void set_nameidata2(struct nameidata *p, struct path *path,
+ struct filename *name, const struct
path *root)
+{
+ __set_nameidata2(p, path, name);
+ p->state = 0;
+ if (unlikely(root)) {
+ p->state = ND_ROOT_PRESET;
+ p->root = *root;
+ }
+}
+
+struct file *do_filp_open_path(struct *path, struct filename *pathname,
+ const struct open_flags *op)
+{
+ struct nameidata nd;
+ int flags = op->lookup_flags;
+ struct file *filp;
+
+ set_nameidata2(&nd, path, 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);
+ restore_nameidata();
+ return filp;
+}
+
struct file *do_file_open_root(const struct path *root,
const char *name, const struct open_flags *op)
{
