linux-kernel - Re: [PATCH 0/1] Fuse Passthrough cache issues

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <d98fefc9-fbb4-44b8-9050-61378176dcf1@fastmail.fm>
Date: Wed, 3 Jul 2024 18:09:01 +0200
From: Bernd Schubert <bernd.schubert@...tmail.fm>
To: Amir Goldstein <amir73il@...il.com>
Cc: Daniel Rosenberg <drosen@...gle.com>, Miklos Szeredi <miklos@...redi.hu>,
 linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
 kernel-team@...roid.com
Subject: Re: [PATCH 0/1] Fuse Passthrough cache issues



On 7/3/24 16:41, Amir Goldstein wrote:
> On Wed, Jul 3, 2024 at 4:27 PM Bernd Schubert
> <bernd.schubert@...tmail.fm> wrote:
>>
>>
>>
>> On 7/3/24 03:02, Daniel Rosenberg wrote:
>>> I've been attempting to recreate Android's usage of Fuse Passthrough with the
>>> version now merged in the kernel, and I've run into a couple issues. The first
>>> one was pretty straightforward, and I've included a patch, although I'm not
>>> convinced that it should be conditional, and it may need to do more to ensure
>>> that the cache is up to date.
>>>
>>> If your fuse daemon is running with writeback cache enabled, writes with
>>> passthrough files will cause problems. Fuse will invalidate attributes on
>>> write, but because it's in writeback cache mode, it will ignore the requested
>>> attributes when the daemon provides them. The kernel is the source of truth in
>>> this case, and should update the cached values during the passthrough write.
>>
>> Could you explain why you want to have the combination passthrough and
>> writeback cache?
>>
>> I think Amirs intention was to have passthrough and cache writes
>> conflicting, see fuse_file_passthrough_open() and
>> fuse_file_cached_io_open().
> 
> Yes, this was an explicit design requirement from Miklos [1].
> I also have use cases to handle some read/writes from server
> and the compromise was that for the first version these cases should
> use FOPEN_DIRECT_IO, which does not conflict with FOPEN_PASSTHROUGH.
> 
> I guess this is not good enough for Android applications opening photos
> that need the FUSE readahead cache for performance?
> 
> In that case, a future BPF filter can decide whether to send the IO direct
> to server or to backing inode.
> 
> Or a future backing inode mapping API could map part of the file to
> backing inode
> and the metadata portion will not be mapped to backing inode will fall back to
> direct IO to server.
> 
> [1] https://lore.kernel.org/linux-fsdevel/CAJfpegtWdGVm9iHgVyXfY2mnR98XJ=6HtpaA+W83vvQea5PycQ@mail.gmail.com/
> 
>>
>> Also in <libfuse>/example/passthrough_hp.cc in sfs_init():
>>
>>     /* Passthrough and writeback cache are conflicting modes */
>>
>>
>>
>> With that I wonder if either fc->writeback_cache should be ignored when
>> a file is opened in passthrough mode, or if fuse_file_io_open() should
>> ignore FOPEN_PASSTHROUGH when fc->writeback_cache is set. Either of both
>> would result in the opposite of what you are trying to achieve - which
>> is why I think it is important to understand what is your actual goal.
>>
> 
> Is there no standard way for FUSE client to tell the server that the
> INIT response is invalid?

Problem is that at FUSE_INIT time it is already mounted. process_init_reply()
can set an error state, but fuse_get_req*() will just result in ECONNREFUSED.*

> 
> Anyway, we already ignore  FUSE_PASSTHROUGH in INIT response
> for several cases, so this could be another case.
> Then FOPEN_PASSTHROUGH will fail with EIO (not be ignored).

So basically this?

diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
index 573550e7bbe1..36c6dcd47a53 100644
--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -1327,7 +1327,8 @@ static void process_init_reply(struct fuse_mount *fm, struct fuse_args *args,
                        if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH) &&
                            (flags & FUSE_PASSTHROUGH) &&
                            arg->max_stack_depth > 0 &&
-                           arg->max_stack_depth <= FILESYSTEM_MAX_STACK_DEPTH) {
+                           arg->max_stack_depth <= FILESYSTEM_MAX_STACK_DEPTH &&
+                           !(flags & FUSE_WRITEBACK_CACHE)) {
                                fc->passthrough = 1;
                                fc->max_stack_depth = arg->max_stack_depth;
                                fm->sb->s_stack_depth = arg->max_stack_depth;




Thanks,
Bernd