On 10/18/23 16:40, Bernd Schubert wrote: > 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... Looking at the android kernel source: /* * For FUSE < 7.36 FUSE_PASSTHROUGH has value (1 << 31). * This condition check is not really required, but would prevent having a * broken commit in the tree. */ #if FUSE_KERNEL_VERSION > 7 || \ (FUSE_KERNEL_VERSION == 7 && FUSE_KERNEL_MINOR_VERSION >= 36) #define FUSE_PASSTHROUGH (1ULL << 63) #else #define FUSE_PASSTHROUGH (1 << 31) #endif So passthrough gets broken with this check and android heavily uses that. Would be interesting to know if this could result in EFAULT. Thanks, Bernd
