Re: [PATCH v2] Revert "fuse: Apply flags2 only when userspace set the FUSE_INIT_EXT"

[Bernd Schubert]

On 10/18/23 16:26, André Draszik wrote:
> On Wed, 2023-10-18 at 11:52 +0000, Bernd Schubert wrote:
>> On 10/18/23 13:46, André Draszik wrote:
>>> On Wed, 2023-10-18 at 11:39 +0000, Bernd Schubert wrote:
>>>> On 10/18/23 13:15, André Draszik wrote:
>>>>> From: André Draszik <andre.draszik@xxxxxxxxxx>
>>>>>
>>>>> This reverts commit 3066ff93476c35679cb07a97cce37d9bb07632ff.
>>>>>
>>>>> This patch breaks all existing userspace by requiring updates
>>>>> as
>>>>> mentioned in the commit message, which is not allowed.
>>>>>
>>>>> Revert to restore compatibility with existing userspace
>>>>> implementations.
>>>>
>>>> Which fuse file system does it exactly break? In fact there
>>>> haven't
>>>> been
>>>> added too many flags after - what exactly is broken?
>>>
>>> The original patch broke the existing kernel <-> user ABI by now
>>> requiring user space applications to pass in an extra flag.
>>> There are various side-effects of this, like unbootable systems,
>>> just
>>> because the kernel was updated.
>>> Breaking the ABI is the one thing that is not allowed. This is not
>>> specific to any particular fuse file system.
>>
>> How exactly did it break it?
> 
> At least in Android, creating new files, or reading existing files
> returns -EFAULT

Hmm, could you please point me to the corresponding android userspace 
library? I guess it is not using libfuse? At least I would like to 
understand the issue...

> 
>>   These are feature flags - is there really a
>> file system that relies on these flag to the extend that it does not
>> work anymore?
> 
> I don't know enough about the implementation details, but even outside
> Android user space had to be updated as a prerequisite for this kernel
> patch:
> https://lore.kernel.org/all/YmUKZQKNAGimupv7@xxxxxxxxxx/
> https://github.com/libfuse/libfuse/pull/662
> 
> Which means any non-Android user space predating those changes isn't
> working anymore either.

The patch in libfuse is from me, there was nothing broken.
And I don't think that any of the additional flags added are a 
_requirement_ for libfuse file systems to work. I'm not sure if DAX and 
the other flags before the patch was merged are a _requirement_ for 
virtiofsd or just a nice feature to have...

In anyway, please still consider that using possibly uninitialized flags 
is not a good idea either and could randomly break things as well.


Thanks,
Bernd