linux-kernel - Re: [PATCH v2] fuse: In fuse_flush only wait if someone wants the return code

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: <Yzb20Y4wHrqUZ93Z@tycho.pizza>
Date:   Fri, 30 Sep 2022 08:01:53 -0600
From:   Tycho Andersen <tycho@...ho.pizza>
To:     Miklos Szeredi <miklos@...redi.hu>
Cc:     Eric Biederman <ebiederm@...ssion.com>,
        "Serge E. Hallyn" <serge@...lyn.com>, linux-kernel@...r.kernel.org,
        fuse-devel@...ts.sourceforge.net
Subject: Re: [PATCH v2] fuse: In fuse_flush only wait if someone wants the
 return code

On Fri, Sep 30, 2022 at 03:35:16PM +0200, Miklos Szeredi wrote:
> On Thu, 29 Sept 2022 at 18:40, Tycho Andersen <tycho@...ho.pizza> wrote:
> >
> > If a fuse filesystem is mounted inside a container, there is a problem
> > during pid namespace destruction. The scenario is:
> >
> > 1. task (a thread in the fuse server, with a fuse file open) starts
> >    exiting, does exit_signals(), goes into fuse_flush() -> wait
> 
> Can't the same happen through
> 
>   fuse_flush -> fuse_sync_writes -> fuse_set_nowrite -> wait
> 
> ?

Looks like yes, though I haven't seen this in the wild, I guess
because there aren't multiple writers most of the time the user code
that causes this.

I'm not exactly sure how to fix this. Reading through 3be5a52b30aa
("fuse: support writable mmap"), we don't want to allow multiple
writes since that may do allocations, which could cause deadlocks. But
in this case we have no reliable way to wait (besides a busy loop, I
suppose).

Maybe just a check for PF_EXITING and a pr_warn() with "echo 1 >
/sys/fs/fuse/connections/$N/abort" or something?

> > +       /*
> > +        * In memory i_blocks is not maintained by fuse, if writeback cache is
> > +        * enabled, i_blocks from cached attr may not be accurate.
> > +        */
> > +       if (!err && fm->fc->writeback_cache)
> > +               fuse_invalidate_attr_mask(fa->inode, STATX_BLOCKS);
> 
> This is still duplicating code, can you please create a helper?

Yep, will do, pending the outcome of the above discussion.

Tycho