mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Seth Forshee <seth.forshee@canonical.com>
To: fuse-devel@lists.sourceforge.net
Cc: Miklos Szeredi <miklos@szeredi.hu>,
	m.loschwitz@syseleven.de, robert@quobyte.com,
	seth.forshee@canonical.com, linux-kernel@vger.kernel.org
Subject: [PATCH 0/2] Fix async io races
Date: Fri, 11 Mar 2016 10:35:32 -0600	[thread overview]
Message-ID: <1457714135-50289-1-git-send-email-seth.forshee@canonical.com> (raw)

These patches are fixes for the problems reported at
https://sourceforge.net/p/fuse/mailman/message/34537139/. The first
patch is one of those that Robert posted on that thread, and the second
is a different patch that Robert and I agree represents a more robust
solution to the race.

The first race is that a is_sync_kiocb() is called on an iocb that could
have been freed if async io has already completed. The fix in this case
is simple and obvious: cache the result before starting io.

In the second case aio completion results in waking a waiting task
followed by unlocking the fuse_io_priv object. The waiting task will
free that object once awoken though, so it's possible for the object to
be freed before spin_unlock() is called. The simple fix is to move the
unlock to before the wake, but really this problem suggests a more
fundamental problem in the lifecycle of fuse_io_priv objects. The
request count in this object is serving two purposes, to count
outstanding aio requests to the fuse server and to serve as a reference
count in the object. For synchronous io these roles come into conflict.
The task which submits io must wait for all requests to the fuse server
to finish and then retrieve the result of the operation from the
fuse_io_priv object, yet it will not be woken until the outstanding
request count reaches 0. This means the task must release its reference
to the object before waiting, which leads to a somewhat confusing set of
rules for when it is safe to free the object.

This becomes much more straightforward if a separate reference count is
maintained for fuse_io_priv objects. Then the waiting task can hold a
reference while waiting, and there is no ambiguity about whether or not
it is safe to free an object. The second patch implements this solution.

Thanks,
Seth

Robert Doebbelin (1):
  fuse: do not use iocb after it may have been freed

Seth Forshee (1):
  fuse: Add reference counting for fuse_io_priv

 fs/fuse/cuse.c   | 12 ++++++++++--
 fs/fuse/file.c   | 47 +++++++++++++++++++++++++++++++++++++----------
 fs/fuse/fuse_i.h | 15 +++++++++++++++
 3 files changed, 62 insertions(+), 12 deletions(-)

             reply	other threads:[~2016-03-11 16:35 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2016-03-11 16:35 Seth Forshee [this message]
2016-03-11 16:35 ` [PATCH 1/2] fuse: do not use iocb after it may have been freed Seth Forshee
2016-03-14 13:54   ` Miklos Szeredi
2016-03-11 16:35 ` [PATCH 2/2] fuse: Add reference counting for fuse_io_priv Seth Forshee
2016-03-14 13:53   ` Miklos Szeredi
2016-03-14 15:12     ` Seth Forshee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1457714135-50289-1-git-send-email-seth.forshee@canonical.com \
    --to=seth.forshee@canonical.com \
    --cc=fuse-devel@lists.sourceforge.net \
    --cc=linux-kernel@vger.kernel.org \
    --cc=m.loschwitz@syseleven.de \
    --cc=miklos@szeredi.hu \
    --cc=robert@quobyte.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®