linux-kernel - Re: [PATCH v5 1/3] FUSE: Avoid lookups in fuse 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: <b8334bd4-8d24-1afa-8809-9b499dd357ea@ddn.com>
Date:   Wed, 18 May 2022 22:28:33 +0200
From:   Bernd Schubert <bschubert@....com>
To:     Vivek Goyal <vgoyal@...hat.com>,
        Dharmendra Singh <dharamhans87@...il.com>
Cc:     miklos@...redi.hu, linux-fsdevel@...r.kernel.org,
        fuse-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org,
        Dharmendra Singh <dsingh@....com>
Subject: Re: [PATCH v5 1/3] FUSE: Avoid lookups in fuse create



On 5/18/22 19:44, Vivek Goyal wrote:
> On Wed, May 18, 2022 at 01:41:02PM -0400, Vivek Goyal wrote:
>> On Tue, May 17, 2022 at 03:37:42PM +0530, Dharmendra Singh wrote:
>>
>> [..]
>>> diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
>>> index d6ccee961891..bebe4be3f1cb 100644
>>> --- a/include/uapi/linux/fuse.h
>>> +++ b/include/uapi/linux/fuse.h
>>> @@ -301,6 +301,7 @@ struct fuse_file_lock {
>>>    * FOPEN_CACHE_DIR: allow caching this directory
>>>    * FOPEN_STREAM: the file is stream-like (no file position at all)
>>>    * FOPEN_NOFLUSH: don't flush data cache on close (unless FUSE_WRITEBACK_CACHE)
>>> + * FOPEN_FILE_CREATED: the file was actually created
>>>    */
>>>   #define FOPEN_DIRECT_IO		(1 << 0)
>>>   #define FOPEN_KEEP_CACHE	(1 << 1)
>>> @@ -308,6 +309,7 @@ struct fuse_file_lock {
>>>   #define FOPEN_CACHE_DIR		(1 << 3)
>>>   #define FOPEN_STREAM		(1 << 4)
>>>   #define FOPEN_NOFLUSH		(1 << 5)
>>> +#define FOPEN_FILE_CREATED	(1 << 6)
>>>   
>>>   /**
>>>    * INIT request/reply flags
>>> @@ -537,6 +539,7 @@ enum fuse_opcode {
>>>   	FUSE_SETUPMAPPING	= 48,
>>>   	FUSE_REMOVEMAPPING	= 49,
>>>   	FUSE_SYNCFS		= 50,
>>> +	FUSE_CREATE_EXT		= 51,
>>
>> I am wondering if we really have to introduce a new opcode for this. Both
>> FUSE_CREATE and FUSE_CREATE_EXT prepare and send fuse_create_in{} and
>> expect fuse_entry_out and fuse_open_out in response. So no new structures
>> are being added. Only thing FUSE_CREATE_EXT does extra is that it also
>> reports back whether file was actually created or not.
>>
>> May be instead of adding an new fuse_opcode, we could simply add a
>> new flag which we send in fuse_create_in and that reqeusts to report
>> if file was created or not. This is along the lines of
>> FUSE_OPEN_KILL_SUIDGID.
>>
>> So say, a new flag FUSE_OPEN_REPORT_CREATE flag. Which we will set in
>> fuse_create_in->open_flags. If file server sees this flag is set, it
>> knows that it needs to set FOPEN_FILE_CREATED flag in response.
>>
>> To me creating a new flag FUSE_OPEN_REPORT_CREATE seems better instead
>> of adding a new opcode.
> 
> Actually I take that back. If we were to use a flag, then we will have to
> do feature negotiation in advance at init time and only then we can set
> FUSE_OPEN_REPORT_CREATE. But we are relying on no new feature bit instead
> -ENOSYS will be returned if server does not support FUSE_CREATE_EXT.
> So adding a new opcode is better.

I guess it might work, if a flag is set and also returned (I would then 
call it FUSE_CREATE_EXT) - user space creat would need to set 
FOPEN_FILE_CREATED and that new flag. I just doubt that it simplifies 
things.

Btw, thanks a lot for your thorough reviews! Much appreciated.


Thanks,
Bernd