linux-kernel - Re: [PATCH v4 1/3] FUSE: Implement atomic lookup + create

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <f512b616-e252-205a-d8e5-4ea7fef53edc@fastmail.fm>
Date:   Wed, 11 May 2022 12:08:53 +0200
From:   Bernd Schubert <bernd.schubert@...tmail.fm>
To:     Jean-Pierre André <jean-pierre.andre@...adoo.fr>,
        linux-fsdevel@...r.kernel.org, Vivek Goyal <vgoyal@...hat.com>,
        Miklos Szeredi <miklos@...redi.hu>
Cc:     fuse-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v4 1/3] FUSE: Implement atomic lookup + create



On 5/7/22 12:42, Jean-Pierre André wrote:
> Bernd Schubert wrote on 5/6/22 8:45 PM:
>>
>>
>> On 5/6/22 19:07, Vivek Goyal wrote:
>>> I looked at fuse_lowlevel API and passthrough_ll.c and there is no
>>> assumption whether FUSE_LOOKUP has already been called by client
>>> before calling FUSE_CREATE. Similarly, I looked at virtiofs code
>>> and I can't see any such assumption there as well.
>>
>> The current linux kernel code does this right now, skipping the lookup 
>> just changes behavior.  Personally I would see it as bug if the 
>> userspace implementation does not handle EEXIST for FUSE_CREATE. 
>> Implementation developer and especially users might see it differently 
>> if a kernel update breaks/changes things out of the sudden. 
>> passthrough_ll.c is not the issue here, it handles it correctly, but 
>> what about the XYZ other file systems out there - do you want to check 
>> them one by one, including closed source ones? And wouldn't even a 
>> single broken application count as regression?
>>
>>>
>>> https://github.com/qemu/qemu/blob/master/tools/virtiofsd/passthrough_ll.c 
>>>
>>>
>>> So I am sort of lost. May be you can help me understsand this.
>>
>> I guess it would be more interesting to look at different file systems 
>> that are not overlay based. Like ntfs-3g - I have not looked at the 
>> code yet, but especially disk based file system didn't have a reason 
>> so far to handle EEXIST. And
> 
> AFAIK ntfs-3g proper does not keep a context across calls and does
> not know what LOOKUP was preparing a CREATE. However this may have
> consequences in the high level of libfuse for managing the file tree.

I don't think high level is effected. I'm really just scared to break

> 
> The kernel apparently issues a LOOKUP to decide whether issuing a
> CREATE (create+open) or an OPEN. If it sent blindly a CREATE,
> ntfs-3g would return EEXIST if the name was already present in
> the directory.

Ok, so ntfs-3g is ok - which leaves N-1 file systems to check...

> 
> For a test, can you suggest a way to force ignore of such lookup
> within libfuse, without applying your kernel patches ? Is there a
> way to detect the purpose of a lookup ? (A possible way is to
> hardcode a directory inode within which the lookups return ENOENT).


What I mean is that we need to check the code or test N file systems - 
if ntfs-3g handles it, N-1 are left...

I we all agree on that there is no issue in skipping the lookup - fine 
with me - it slightly simplifies the patches.


Thanks,
Bernd