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: <170233343177.12910.2316815312951521227@noble.neil.brown.name>
Date:   Tue, 12 Dec 2023 09:23:51 +1100
From:   "NeilBrown" <neilb@...e.de>
To:     "Al Viro" <viro@...iv.linux.org.uk>
Cc:     "Chuck Lever" <chuck.lever@...cle.com>,
        "Christian Brauner" <brauner@...nel.org>,
        "Jens Axboe" <axboe@...nel.dk>, "Oleg Nesterov" <oleg@...hat.com>,
        "Jeff Layton" <jlayton@...nel.org>, linux-fsdevel@...r.kernel.org,
        linux-kernel@...r.kernel.org, linux-nfs@...r.kernel.org
Subject: Re: [PATCH 1/3] nfsd: use __fput_sync() to avoid delayed closing of files.

On Tue, 12 Dec 2023, Al Viro wrote:
> On Mon, Dec 11, 2023 at 09:47:35AM +1100, NeilBrown wrote:
> 
> > Similarly would could wrap __fput_sync() is a more friendly name, but
> > that would be better if we actually renamed the function.
> > 
> >   void fput_now(struct file *f)
> >   {
> >       __fput_sync(f);
> >   }
> 
> It is unfriendly *precisely* because it should not be used without
> a very good reason.
> 
> It may be the last opened file keeping a lazy-umounted mount alive.
> It may be taking pretty much any locks, or eating a lot of stack
> space.

Previously you've suggested problems with ->release blocking.
Now you refer to lazy-umount, which is what the comment above
__fput_sync() mentions.

"pretty much an locks" seems like hyperbole.  I don't see it taking
nfsd_mutex or nlmsvc_mutex.  Maybe you mean any filesystem lock?

My understanding is that the advent of vmalloc allocated stacks means
that kernel stack space is not an important consideration.

It would really help if we could have clear documented explanation of
what problems can occur.  Maybe an example of contexts where it isn't
safe to call __fput_sync().

I can easily see that lazy-unmount is an interesting case which could
easily catch people unawares.  Punting the tail end of mntput_no_expire
(i.e.  if count reaches zero) to a workqueue/task_work makes sense and
would be much less impact than punting every __fput to a workqueue.

Would that make an fput_now() call safe to use in most contexts, or is
there something about ->release or dentry_kill() that can still cause
problems?

Thanks,
NeilBrown


> 
> It really isn't a general-purpose API; any "more friendly name"
> is going to be NAKed for that reason alone.
> 
> Al, very much tempted to send a patch renaming that sucker to
> __fput_dont_use_that_unless_you_really_know_what_you_are_doing().
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ