| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <CAOQ4uxgzThRacOhcwQcU6DAx7MEUc-8-Z6j9fSKzJp+kuc5=-Q@mail.gmail.com>
Date: Fri, 21 Nov 2025 11:53:12 +0100
From: Amir Goldstein <amir73il@...il.com>
To: Luis Henriques <luis@...lia.com>
Cc: Miklos Szeredi <miklos@...redi.hu>, "Darrick J. Wong" <djwong@...nel.org>,
Bernd Schubert <bschubert@....com>, Kevin Chen <kchen@....com>,
Horst Birthelmer <hbirthelmer@....com>, linux-fsdevel <linux-fsdevel@...r.kernel.org>,
linux-kernel <linux-kernel@...r.kernel.org>, Matt Harvey <mharvey@...ptrading.com>,
kernel-dev@...lia.com
Subject: Re: [RFC PATCH v1 4/3] fuse: implementation of export_operations with FUSE_LOOKUP_HANDLE
[changing the subject to comment on this part separately]
On Thu, Nov 20, 2025 at 11:55 AM Luis Henriques <luis@...lia.com> wrote:
>
>
> The export_operations were also modified to use this new file handle instead
> if the lookup_handle operation is implemented for the file system.
>
> Signed-off-by: Luis Henriques <luis@...lia.com>
...
>
> +enum {
> + HANDLE_TYPE_NODEID = 0,
> + HANDLE_TYPE_HANDLE = 1,
> +};
> +
> struct fuse_inode_handle {
> - u64 nodeid;
> - u32 generation;
> + u32 type;
I don't understand the reason for type as it is always categorically
determined by fc->lookup_handle in this code.
> + union {
Perhaps not a union, see below...
> + struct {
> + u64 nodeid;
> + u32 generation;
> + };
> + struct fuse_file_handle fh;
Feels like this should be struct fuse_file_handle *fh;
> + };
> };
>
> static struct dentry *fuse_get_dentry(struct super_block *sb,
> @@ -1092,7 +1147,7 @@ static struct dentry *fuse_get_dentry(struct super_block *sb,
> goto out_err;
> }
>
> - err = fuse_lookup_name(sb, handle->nodeid, &name, outarg,
> + err = fuse_lookup_name(sb, handle->nodeid, NULL, &name, outarg,
> &inode);
This is a special case of LOOKUP where the parent is unknown.
Current fuse code does lookup for name "." in "parent" nodeid and servers
should know how to treat it specially.
I think that for LOOKUP_HANDLE, we can do one of two things.
Either we always encode the nodeid in NFS exported file handles
in addition to the server file handle,
or we skip the ilookup5() optimization and alway send LOOKUP_HANDLE
to the fuse server with nodeid 0 when the NFS server calls fuse_fh_to_dentry()
so the server knows to lookup only by file handle.
The latter option sounds more robust, OTOH the ilookup5() "optimization"
could be quite useful, so not sure it is worth giving up on.
> kfree(outarg);
> if (err && err != -ENOENT)
> @@ -1121,13 +1176,42 @@ static struct dentry *fuse_get_dentry(struct super_block *sb,
> return ERR_PTR(err);
> }
>
> +static int fuse_encode_lookup_fh(struct inode *inode, u32 *fh, int *max_len,
> + struct inode *parent)
> +{
> + struct fuse_inode *fi = get_fuse_inode(inode);
> + int total_len, len;
> +
> + total_len = len = sizeof(struct fuse_file_handle);
> + if (parent)
> + total_len *= 2;
> +
> + if (*max_len < total_len)
> + return FILEID_INVALID;
> +
> + memcpy(fh, &fi->fh, len);
> + if (parent) {
> + fi = get_fuse_inode(parent);
> + memcpy((fh + len), &fi->fh, len);
> + }
> +
> + *max_len = total_len;
> +
> + /* XXX define new fid_type */
> + return parent ? FILEID_INO64_GEN_PARENT : FILEID_INO64_GEN;
> +}
> +
This is very odd.
I don't understand what you were trying to do here.
As far as I can see, you are not treating fuse_inode_handle as a variable
length struct that it is in the export operations.
> static int fuse_encode_fh(struct inode *inode, u32 *fh, int *max_len,
> struct inode *parent)
> {
> + struct fuse_conn *fc = get_fuse_conn(inode);
> int len = parent ? 6 : 3;
> u64 nodeid;
> u32 generation;
>
> + if (fc->lookup_handle)
> + return fuse_encode_lookup_fh(inode, fh, max_len, parent);
> +
> if (*max_len < len) {
> *max_len = len;
> return FILEID_INVALID;
> @@ -1156,30 +1240,51 @@ static int fuse_encode_fh(struct inode *inode, u32 *fh, int *max_len,
> static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
> struct fid *fid, int fh_len, int fh_type)
> {
> + struct fuse_conn *fc = get_fuse_conn_super(sb);
> struct fuse_inode_handle handle;
>
> if ((fh_type != FILEID_INO64_GEN &&
> fh_type != FILEID_INO64_GEN_PARENT) || fh_len < 3)
> return NULL;
>
> - handle.nodeid = (u64) fid->raw[0] << 32;
> - handle.nodeid |= (u64) fid->raw[1];
> - handle.generation = fid->raw[2];
> + if (fc->lookup_handle) {
> + if (fh_len < sizeof(struct fuse_file_handle))
> + return NULL;
> + handle.type = HANDLE_TYPE_HANDLE;
> + memcpy(&handle.fh, &fid->raw[0],
> + sizeof(struct fuse_file_handle));
> + } else {
> + handle.nodeid = (u64) fid->raw[0] << 32;
> + handle.nodeid |= (u64) fid->raw[1];
> + handle.generation = fid->raw[2];
> + }
> return fuse_get_dentry(sb, &handle);
> }
>
> static struct dentry *fuse_fh_to_parent(struct super_block *sb,
> struct fid *fid, int fh_len, int fh_type)
> {
> - struct fuse_inode_handle parent;
> + struct fuse_conn *fc = get_fuse_conn_super(sb);
> + struct fuse_inode_handle handle;
>
> if (fh_type != FILEID_INO64_GEN_PARENT || fh_len < 6)
> return NULL;
>
> - parent.nodeid = (u64) fid->raw[3] << 32;
> - parent.nodeid |= (u64) fid->raw[4];
> - parent.generation = fid->raw[5];
> - return fuse_get_dentry(sb, &parent);
> + if (fc->lookup_handle) {
> + struct fuse_file_handle *fh = (struct fuse_file_handle *)fid->raw;
> +
> + if (fh_len < sizeof(struct fuse_file_handle) * 2)
> + return NULL;
> + handle.type = HANDLE_TYPE_HANDLE;
> + memcpy(&handle.fh, &fh[1],
> + sizeof(struct fuse_file_handle));
> + } else {
> + handle.type = HANDLE_TYPE_NODEID;
> + handle.nodeid = (u64) fid->raw[3] << 32;
> + handle.nodeid |= (u64) fid->raw[4];
> + handle.generation = fid->raw[5];
> + }
> + return fuse_get_dentry(sb, &handle);
> }
>
You may want to look at ovl_encode_fh() as an example of how overlayfs
encapsulates whatever file handle it got from the real filesystem and packs it
as type OVL_FILEID_V1 to hand out to the NFS server.
If we go that route, then the FILEID_FUSE type may include the legacy
nodeid+gen and the server's variable length file handle following that.
To support "connectable" file handles (see AT_HANDLE_CONNECTABLE
and fstests test generic/777) would need to implement also handle type
FILEID_FUSE_WITH_PARENT, which is a concatenation of two
variable sized FILEID_FUSE handles.
But note that the fact that the server supports LOOKUP_HANDLE does not
mean that all NFS exported handles MUST be FILEID_FUSE handles.
I think we consider allowing the server to reply to LOOKUP_HANDLE without
a file handle argument (only nodeid+gen) for specific inodes (e.g. the
root inode).
If we allow that, then those inodes could still be encoded as FILEID_INO64_GEN
when exported to NFS and when fuse_fh_to_dentry() is requested to decide
a file handle of type FILEID_INO64_GEN, it may call LOOKUP_HANDLE
without nodeid and without a file handle and let the fuse server decide if this
lookup is acceptable (e.g. with FUSE_ROOT_ID) or stale.
One thing that would be useful with the above is that you will not
have to implement
encode/decode of FILEID_FUSE for the first version of LOOKUP_HANDLE.
First of all you could require FUSE_NO_EXPORT_SUPPORT for first version,
but even without it, a fuse server that supports LOOKUP_HANDLE can fail
LOOKUP_HANDLE requests without a file handle (or FUSE_ROOT_ID) and
even that will behave better than NFS export of fuse today (*).
Hope I was not piling too much and that I was not piling garbage.
Thanks,
Amir.
(*) In current fuse, fuse_fh_to_dentry() after fuse was unmounted and mounted
may return ESTALE if nodeid is not already in inode cache and it may also
decode the wrong object if after fuse restart nodeid (with same gen)
was assigned
to a completely different object (yes that happens).
Powered by blists - more mailing lists