Linux-Fsdevel Archive on lore.kernel.org
help / color / mirror / Atom feed
From: Krzysztof Rusek <rusek@9livesdata.com>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: linux-fsdevel@vger.kernel.org
Subject: Re: fuse_notify_inval_inode() may be ineffective when getattr request is in progress
Date: Wed, 20 May 2020 14:23:58 +0200	[thread overview]
Message-ID: <d9459e74-92b4-187b-4b73-bd807e79d813@9livesdata.com> (raw)
In-Reply-To: <CAJfpegs+auq0TQ4SaFiLb7w9E+ksWHCzgBoOhCCFGF6R9DMFdA@mail.gmail.com>

[-- Attachment #1: Type: text/plain, Size: 2446 bytes --]

Hi Miklos,

I've checked that after applying the patch, the problem no longer occurs.

Since I'm running tests on RedHat 7.7, I had to slightly modify the 
patch, because on that kernel version locking around attr_version is 
done differently. I'm attaching modified patch, just in case.

Best regards,
Krzysztof Rusek

On 5/18/20 3:08 PM, Miklos Szeredi wrote:
> On Mon, May 4, 2020 at 1:00 PM Krzysztof Rusek <rusek@9livesdata.com> wrote:
>>
>> Hello,
>>
>> I'm working on a user-space file system implementation utilizing fuse
>> kernel module (and libfuse user-space library). This file system
>> implementation provides a custom ioctl operation that uses
>> fuse_lowlevel_notify_inval_inode() function (which translates to
>> fuse_notify_inval_inode() in kernel) to notify the kernel that the file
>> was changed by the ioctl. However, under certain circumstances,
>> fuse_notify_inval_inode() call is ineffective, resulting in incorrect
>> file attributes being cached by the kernel. The problem occurs when
>> ioctl() is executed in parallel with getattr().
>>
>> I noticed this problem on RedHat 7.7 (3.10.0-1062.el7.x86_64), but I
>> believe mainline kernel is affected as well.
>>
>> I think there is a bug in the way fuse_notify_inval_inode() invalidates
>> file attributes. If getattr() started before fuse_notify_inval_inode()
>> was called, then the attributes returned by user-space process may be
>> out of date, and should not be cached. But fuse_notify_inval_inode()
>> only clears attribute validity time, which does not prevent getattr()
>> result from being cached.
>>
>> More precisely, this bug occurs in the following scenario:
>>
>> 1. kernel starts handling ioctl
>> 2. file system process receives ioctl request
>> 3. kernel starts handling getattr
>> 4. file system process receives getattr request
>> 5. file system process executes getattr
>> 6. file system process executes ioctl, changing file state
>> 7. file system process invokes fuse_lowlevel_notify_inval_inode(), which
>> invalidates file attributes in kernel
>> 8. file system process sends ioctl reply, ioctl handling ends
>> 9. file system process sends getattr reply, attributes are incorrectly
>> cached in the kernel
>>
>> (Note that this scenario assumes that file system implementation is
>> multi-threaded, therefore allowing ioctl() and getattr() to be handled
>> in parallel.)
> 
> Can you please try the attached patch?
> 
> Thanks,
> Miklos
> 

[-- Attachment #2: fuse-update-attr_version-counter-on-fuse_notify_inval_inode.patch.el7.diff --]
[-- Type: text/x-patch, Size: 703 bytes --]

--- a/fs/fuse/inode.c
+++ b/fs/fuse/inode.c
@@ -332,6 +332,8 @@ struct inode *fuse_iget(struct super_block *sb, u64 nodeid,
 int fuse_reverse_inval_inode(struct super_block *sb, u64 nodeid,
 			     loff_t offset, loff_t len)
 {
+	struct fuse_conn *fc = get_fuse_conn_super(sb);
+	struct fuse_inode *fi;
 	struct inode *inode;
 	pgoff_t pg_start;
 	pgoff_t pg_end;
@@ -340,6 +342,11 @@ int fuse_reverse_inval_inode(struct super_block *sb, u64 nodeid,
 	if (!inode)
 		return -ENOENT;
 
+	fi = get_fuse_inode(inode);
+	spin_lock(&fc->lock);
+	fi->attr_version = ++fc->attr_version;
+	spin_unlock(&fc->lock);
+
 	fuse_invalidate_attr(inode);
 	if (offset >= 0) {
 		pg_start = offset >> PAGE_CACHE_SHIFT;

  reply	other threads:[~2020-05-20 12:24 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-04 11:00 Krzysztof Rusek
2020-05-18 13:08 ` Miklos Szeredi
2020-05-20 12:23   ` Krzysztof Rusek [this message]
2020-05-20 12:26     ` Miklos Szeredi

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=d9459e74-92b4-187b-4b73-bd807e79d813@9livesdata.com \
    --to=rusek@9livesdata.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    --subject='Re: fuse_notify_inval_inode() may be ineffective when getattr request is in progress' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).