Re: [PATCH] fuse: Apply flags2 only when userspace set the FUSE_INIT_EXT flag

[JeffleXu <jefflexu@linux.alibaba.com>]

On 4/24/22 6:49 PM, Bernd Schubert wrote:
> I'm also traveling, but I had checked a bit the links you had given and even created github issue for the rust-fuse because it uses conflicting flags - seems to rely on non-upstream kernel.

FYI at least the C version virtiofsd (git@github.com:qemu/qemu.git
master) doesn't set FUSE_INIT_EXT on the reply to FUSE_INIT. I didn't
check the Rust version, since I'm not familiar with Rust so far...

I guess Vivek was referring to [1] when he mentioned the rust version of
virtiofsd. This is the rust version developed by the RedHat.

As for the "rust-fuse" Bernd Schubert mentioned, actually it's [2]
developed by the Alibaba folks. We tried to make this fuse daemon
support the per-inode DAX feature when the feature is still in the
progress of upstreaming kernel. Later when the feature finally gets
merged to mainline kernel, the position of the FUSE_HAS_INODE_DAX flag
bit is a little different with the initial implementation. Sadly we
forget to fix this, and the fuse daemon keeps using the flag bit
different from the mainline version. Sorry for that. Thanks for pointing
it out and we are going to fix it. Thanks.


[1] https://gitlab.com/virtio-fs/virtiofsd
[2] https://github.com/cloud-hypervisor/fuse-backend-rs


> 
> Get Outlook for Android<https://aka.ms/AAb9ysg>
> ________________________________
> From: Vivek Goyal <vgoyal@redhat.com>
> Sent: Sunday, April 24, 2022 10:29:25 AM
> To: Miklos Szeredi <miklos@szeredi.hu>
> Cc: Bernd Schubert <bschubert@ddn.com>; linux-fsdevel@vger.kernel.org <linux-fsdevel@vger.kernel.org>; Jeffle Xu <jefflexu@linux.alibaba.com>; Dharmendra Singh <dsingh@ddn.com>; Dr. David Alan Gilbert <dgilbert@redhat.com>; German Maglione <gmaglione@redhat.com>
> Subject: Re: [PATCH] fuse: Apply flags2 only when userspace set the FUSE_INIT_EXT flag
> 
> On Thu, Apr 21, 2022 at 05:36:02PM +0200, Miklos Szeredi wrote:
>> On Fri, 15 Apr 2022 at 13:54, Bernd Schubert <bschubert@ddn.com> wrote:
>>>
>>> This is just a safety precaution to avoid checking flags
>>> on memory that was initialized on the user space side.
>>> libfuse zeroes struct fuse_init_out outarg, but this is not
>>> guranteed to be done in all implementations. Better is to
>>> act on flags and to only apply flags2 when FUSE_INIT_EXT
>>> is set.
>>>
>>> There is a risk with this change, though - it might break existing
>>> user space libraries, which are already using flags2 without
>>> setting FUSE_INIT_EXT.
>>>
>>> The corresponding libfuse patch is here
>>> https://github.com/libfuse/libfuse/pull/662
>>>
>>>
>>> Signed-off-by: Bernd Schubert <bschubert@ddn.com>
>>
>> Agreed, this is a good change.  Applied.
>>
>> Just one comment: please consider adding  "Fixes:" and "Cc:
>> <stable@....>" tags next time.   I added them now.
> 
> I am afraid that this probably will break both C and rust version of
> virtiofsd. I had a quick look and I can't seem to find these
> implementations setting INIT_EXT flag in reply to init.
> 
> I am travelling. Will check it more closely when I return next week.
> If virtiofsd implementations don't set INIT_EXT, I would rather prefer
> to not do this change and avoid breaking it.
> 
> Thanks
> Vivek
> 
> 

-- 
Thanks,
Jeffle