| 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: <87zf7ibans.fsf@wotan.olymp>
Date: Tue, 16 Dec 2025 17:26:31 +0000
From: Luis Henriques <luis@...lia.com>
To: Amir Goldstein <amir73il@...il.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@...r.kernel.org, linux-kernel@...r.kernel.org, Matt
Harvey <mharvey@...ptrading.com>, kernel-dev@...lia.com
Subject: Re: [RFC PATCH v2 6/6] fuse: implementation of export_operations
with FUSE_LOOKUP_HANDLE
On Tue, Dec 16 2025, Amir Goldstein wrote:
> On Fri, Dec 12, 2025 at 7:13 PM Luis Henriques <luis@...lia.com> wrote:
>>
>> This patch allows the NFS handle to use the new file handle provided by the
>> LOOKUP_HANDLE operation. It still allows the usage of nodeid+generation as
>> an handle if this operation is not supported by the FUSE server or if no
>> handle is available for a specific inode. I.e. it can mix both file handle
>> types FILEID_INO64_GEN{_PARENT} and FILEID_FUSE_WITH{OUT}_PARENT.
>
> Why the mix? I dont get it.
Well, my thought was that if we have a nodeid for which we don't have a
file handle (FUSE_NODE_ID is the only example I can think of), we still
need to encode it. And the idea was to use the old FILEID_INO64_GEN in
those cases, even if all other nodeids use the new FILEID. But I guess we
can simply use the nodeid+gen file handle with the new FILEID anyway.
>>
>> Signed-off-by: Luis Henriques <luis@...lia.com>
>> ---
>> fs/fuse/export.c | 162 ++++++++++++++++++++++++++++++++++++---
>> include/linux/exportfs.h | 7 ++
>> 2 files changed, 160 insertions(+), 9 deletions(-)
>>
>> diff --git a/fs/fuse/export.c b/fs/fuse/export.c
>> index 4a9c95fe578e..b40d146a32f2 100644
>> --- a/fs/fuse/export.c
>> +++ b/fs/fuse/export.c
>> @@ -3,6 +3,7 @@
>> * FUSE NFS export support.
>> *
>> * Copyright (C) 2001-2008 Miklos Szeredi <miklos@...redi.hu>
>> + * Copyright (C) 2025 Jump Trading LLC, author: Luis Henriques <luis@...lia.com>
>> */
>>
>> #include "fuse_i.h"
>> @@ -10,7 +11,8 @@
>>
>> struct fuse_inode_handle {
>> u64 nodeid;
>> - u32 generation;
>> + u32 generation; /* XXX change to u64, and use fid->i64.ino in encode/decode? */
>> + struct fuse_file_handle fh;
>
> If anything this should be a union
> or maybe I don't understand what you were trying to accomplish.
> Please try to explain the design better in the commit message.
As Miklos also suggested using either nodeid+gen or the new fh, I guess it
makes sense to use a union here.
>> };
>>
>> static struct dentry *fuse_get_dentry(struct super_block *sb,
>> @@ -67,8 +69,8 @@ static struct dentry *fuse_get_dentry(struct super_block *sb,
>> return ERR_PTR(err);
>> }
>>
>> -static int fuse_encode_fh(struct inode *inode, u32 *fh, int *max_len,
>> - struct inode *parent)
>> +static int fuse_encode_gen_fh(struct inode *inode, u32 *fh, int *max_len,
>> + struct inode *parent)
>> {
>> int len = parent ? 6 : 3;
>> u64 nodeid;
>> @@ -96,38 +98,180 @@ static int fuse_encode_fh(struct inode *inode, u32 *fh, int *max_len,
>> }
>>
>> *max_len = len;
>> +
>> return parent ? FILEID_INO64_GEN_PARENT : FILEID_INO64_GEN;
>> }
>>
>> -static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
>> - struct fid *fid, int fh_len, int fh_type)
>> +static int fuse_encode_fuse_fh(struct inode *inode, u32 *fh, int *max_len,
>> + struct inode *parent)
>> +{
>> + struct fuse_inode *fi = get_fuse_inode(inode);
>> + struct fuse_inode *fip = NULL;
>> + struct fuse_inode_handle *handle = (void *)fh;
>> + int type = FILEID_FUSE_WITHOUT_PARENT;
>> + int len, lenp = 0;
>> + int buflen = *max_len << 2; /* max_len: number of words */
>> +
>> + len = sizeof(struct fuse_inode_handle) + fi->fh->size;
>> + if (parent) {
>> + fip = get_fuse_inode(parent);
>> + if (fip->fh && fip->fh->size) {
>> + lenp = sizeof(struct fuse_inode_handle) +
>> + fip->fh->size;
>> + type = FILEID_FUSE_WITH_PARENT;
>> + }
>> + }
>> +
>> + if (buflen < (len + lenp)) {
>> + *max_len = (len + lenp) >> 2;
>> + return FILEID_INVALID;
>> + }
>> +
>> + handle[0].nodeid = fi->nodeid;
>> + handle[0].generation = inode->i_generation;
>> + memcpy(&handle[0].fh, fi->fh, len);
>> + if (lenp) {
>> + handle[1].nodeid = fip->nodeid;
>> + handle[1].generation = parent->i_generation;
>> + memcpy(&handle[1].fh, fip->fh, lenp);
>> + }
>> +
>> + *max_len = (len + lenp) >> 2;
>> +
>> + return type;
>> +}
>> +
>> +static int fuse_encode_fh(struct inode *inode, u32 *fh, int *max_len,
>> + struct inode *parent)
>> +{
>> + struct fuse_conn *fc = get_fuse_conn(inode);
>> + struct fuse_inode *fi = get_fuse_inode(inode);
>> +
>> + if (fc->lookup_handle && fi->fh && fi->fh->size)
>> + return fuse_encode_fuse_fh(inode, fh, max_len, parent);
>> +
>> + return fuse_encode_gen_fh(inode, fh, max_len, parent);
>> +}
>> +
>> +static struct dentry *fuse_fh_gen_to_dentry(struct super_block *sb,
>> + struct fid *fid, int fh_len)
>> {
>> struct fuse_inode_handle handle;
>>
>> - if ((fh_type != FILEID_INO64_GEN &&
>> - fh_type != FILEID_INO64_GEN_PARENT) || fh_len < 3)
>> + if (fh_len < 3)
>
> I dont understand why this was changed.
The reason is because fuse_fh_to_dentry() is the only caller of this
function. Since we already know the type is correct (fuse_fh_to_dentry()
checks it), there's not point checking it again here.
>> return NULL;
>>
>> 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,
>> +static struct dentry *fuse_fh_fuse_to_dentry(struct super_block *sb,
>> + struct fid *fid, int fh_len)
>> +{
>> + struct fuse_inode_handle *handle;
>> + struct dentry *dentry;
>> + int len = sizeof(struct fuse_file_handle);
>> +
>> + handle = (void *)fid;
>> + len += handle->fh.size;
>> +
>> + if ((fh_len << 2) < len)
>> + return NULL;
>> +
>> + handle = kzalloc(len, GFP_KERNEL);
>> + if (!handle)
>> + return NULL;
>> +
>> + memcpy(handle, fid, len);
>> +
>> + dentry = fuse_get_dentry(sb, handle);
>> + kfree(handle);
>> +
>> + return dentry;
>> +}
>> +
>> +static struct dentry *fuse_fh_to_dentry(struct super_block *sb,
>> struct fid *fid, int fh_len, int fh_type)
>> +{
>> + switch (fh_type) {
>> + case FILEID_INO64_GEN:
>> + case FILEID_INO64_GEN_PARENT:
>> + return fuse_fh_gen_to_dentry(sb, fid, fh_len);
>> + case FILEID_FUSE_WITHOUT_PARENT:
>> + case FILEID_FUSE_WITH_PARENT:
>> + return fuse_fh_fuse_to_dentry(sb, fid, fh_len);
>> + }
>> +
>> + return NULL;
>> +
>> +}
>> +
>> +static struct dentry *fuse_fh_gen_to_parent(struct super_block *sb,
>> + struct fid *fid, int fh_len)
>> {
>> struct fuse_inode_handle parent;
>>
>> - if (fh_type != FILEID_INO64_GEN_PARENT || fh_len < 6)
>> + if (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);
>> }
>>
>> +static struct dentry *fuse_fh_fuse_to_parent(struct super_block *sb,
>> + struct fid *fid, int fh_len)
>> +{
>> + struct fuse_inode_handle *handle;
>> + struct dentry *dentry;
>> + int total_len;
>> + int len;
>> +
>> + handle = (void *)fid;
>> + total_len = len = sizeof(struct fuse_inode_handle) + handle->fh.size;
>> +
>> + if ((fh_len << 2) < total_len)
>> + return NULL;
>> +
>> + handle = (void *)(fid + len);
>> + len = sizeof(struct fuse_file_handle) + handle->fh.size;
>> + total_len += len;
>> +
>> + if ((fh_len << 2) < total_len)
>> + return NULL;
>> +
>> + handle = kzalloc(len, GFP_KERNEL);
>> + if (!handle)
>> + return NULL;
>> +
>> + memcpy(handle, fid, len);
>> +
>> + dentry = fuse_get_dentry(sb, handle);
>> + kfree(handle);
>> +
>> + return dentry;
>> +}
>> +
>> +static struct dentry *fuse_fh_to_parent(struct super_block *sb,
>> + struct fid *fid, int fh_len, int fh_type)
>> +{
>> + switch (fh_type) {
>> + case FILEID_INO64_GEN:
>> + case FILEID_INO64_GEN_PARENT:
>> + return fuse_fh_gen_to_parent(sb, fid, fh_len);
>> + case FILEID_FUSE_WITHOUT_PARENT:
>> + case FILEID_FUSE_WITH_PARENT:
>> + return fuse_fh_fuse_to_parent(sb, fid, fh_len);
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> static struct dentry *fuse_get_parent(struct dentry *child)
>> {
>> struct inode *child_inode = d_inode(child);
>> diff --git a/include/linux/exportfs.h b/include/linux/exportfs.h
>> index f0cf2714ec52..db783f6b28bc 100644
>> --- a/include/linux/exportfs.h
>> +++ b/include/linux/exportfs.h
>> @@ -110,6 +110,13 @@ enum fid_type {
>> */
>> FILEID_INO64_GEN_PARENT = 0x82,
>>
>> + /*
>> + * 64 bit nodeid number, 32 bit generation number,
>> + * variable length handle.
>> + */
>> + FILEID_FUSE_WITHOUT_PARENT = 0x91,
>> + FILEID_FUSE_WITH_PARENT = 0x92,
>> +
>
>
> Do we even need a handle with inode+gen+server specific handle?
> I didn't think we would. If we do, please explain why.
I *think* I've addressed this my other comments above. Basically, I agree
that we don't need it. FUSE_ROOT_ID would be the only exception, but
there's no need for this nodeid+gen+fh just because of that. (Or maybe I
misunderstood this comment?)
(Oh, and thanks a lot everyone for the comments! Much appreciated.)
Cheers,
--
Luís
Powered by blists - more mailing lists