linux-kernel - Re: [RFC PATCH 7/9] fuse: add fuse device ioctl(FUSE_DEV_IOC

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] [day] [month] [year] [list]

Message-ID: <4632a71e-8f15-66ad-3193-4605a1204f8d@ddn.com>
Date:   Tue, 11 Apr 2023 22:18:11 +0000
From:   Bernd Schubert <bschubert@....com>
To:     Aleksandr Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>
CC:     "mszeredi@...hat.com" <mszeredi@...hat.com>,
        Al Viro <viro@...iv.linux.org.uk>,
        Amir Goldstein <amir73il@...il.com>,
        Stéphane Graber <stgraber@...ntu.com>,
        Seth Forshee <sforshee@...nel.org>,
        Christian Brauner <brauner@...nel.org>,
        Andrei Vagin <avagin@...il.com>,
        Pavel Tikhomirov <ptikhomirov@...tuozzo.com>,
        "linux-fsdevel@...r.kernel.org" <linux-fsdevel@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "criu@...nvz.org" <criu@...nvz.org>
Subject: Re: [RFC PATCH 7/9] fuse: add fuse device ioctl(FUSE_DEV_IOC_REINIT)



On 4/3/23 16:51, Aleksandr Mikhalitsyn wrote:
> On Mon, Mar 6, 2023 at 3:09 PM Aleksandr Mikhalitsyn
> <aleksandr.mikhalitsyn@...onical.com> wrote:
>>
>> On Fri, Mar 3, 2023 at 8:26 PM Bernd Schubert
>> <bernd.schubert@...tmail.fm> wrote:
>>>
>>>
>>>
>>> On 2/20/23 20:37, Alexander Mikhalitsyn wrote:
>>>> This ioctl aborts fuse connection and then reinitializes it,
>>>> sends FUSE_INIT request to allow a new userspace daemon
>>>> to pick up the fuse connection.
>>>>
>>>> Cc: Miklos Szeredi <mszeredi@...hat.com>
>>>> Cc: Al Viro <viro@...iv.linux.org.uk>
>>>> Cc: Amir Goldstein <amir73il@...il.com>
>>>> Cc: StÃ©phane Graber <stgraber@...ntu.com>
>>>> Cc: Seth Forshee <sforshee@...nel.org>
>>>> Cc: Christian Brauner <brauner@...nel.org>
>>>> Cc: Andrei Vagin <avagin@...il.com>
>>>> Cc: Pavel Tikhomirov <ptikhomirov@...tuozzo.com>
>>>> Cc: linux-fsdevel@...r.kernel.org
>>>> Cc: linux-kernel@...r.kernel.org
>>>> Cc: criu@...nvz.org
>>>> Signed-off-by: Alexander Mikhalitsyn <aleksandr.mikhalitsyn@...onical.com>
>>>> ---
>>>>    fs/fuse/dev.c             | 132 ++++++++++++++++++++++++++++++++++++++
>>>>    include/uapi/linux/fuse.h |   1 +
>>>>    2 files changed, 133 insertions(+)
>>>>
>>>> diff --git a/fs/fuse/dev.c b/fs/fuse/dev.c
>>>> index 737764c2295e..0f53ffd63957 100644
>>>> --- a/fs/fuse/dev.c
>>>> +++ b/fs/fuse/dev.c
>>>> @@ -2187,6 +2187,112 @@ void fuse_abort_conn(struct fuse_conn *fc)
>>>>    }
>>>>    EXPORT_SYMBOL_GPL(fuse_abort_conn);
>>>>
>>>> +static int fuse_reinit_conn(struct fuse_conn *fc)
>>>> +{
>>>> +     struct fuse_iqueue *fiq = &fc->iq;
>>>> +     struct fuse_dev *fud;
>>>> +     unsigned int i;
>>>> +
>>>> +     if (fc->conn_gen + 1 < fc->conn_gen)
>>>> +             return -EOVERFLOW;
>>>> +
>>>> +     fuse_abort_conn(fc);
>>>> +     fuse_wait_aborted(fc);
>>>
>>> Shouldn't this also try to flush all data first?
> 
> Dear Bernd,
> 
> I've reviewed this place 2nd time and I'm not sure that we have to
> perform any flushing there, because userspace daemon can be dead or
> stuck.
> Technically, if userspace knows that daemon is alive then it can call
> fsync/sync before doing reinit.
> 
> What do you think about it?

Hello Alex,

sorry for my late reply.

Hmm, I just fear that fsync/sync is a bit racy, what is if a user would 
write data after the sync and that would get silently removed by 
fuse_abort_conn()? Isn't what we want:

ioctl
    refuse new requests -> unset fc->initialized
    flush all fc queues (fc->iq.pending, fc->bg_queue, I guess with your 
current patches we do not need to handle forget)
    fuse_abort_conn


So what is missing is the information if the daemon is still running - 
take a daemon reference and then check for PF_EXITING, as in my uring 
patches? Miklos has some objections for that, though.


The alternative would be to mount read-only, then sync, then do the 
ioctl and remount back. I don't know what needs to be done to get 
remount working, though. Just handle it in libfuse mount.fuse and send 
the mount syscall?


As I wrote before, at DDN we want to have run time daemon restart - I'm 
also not opposed to entirely give up on the flush and to just work on a 
restart protocol to make the new daemon to the old state (opened files 
and lookup/forget count). In principle we could even transfer that in 
userspace from one daemon to the other?


Thanks,
Bernd

PS: Will look at the new patches later this week.


Thanks,
Bernd