Re: [PATCH] fuse: fix illegal access to inode with reused nodeid

From: Amir Goldstein <amir73il@gmail.com>
To: Nikolaus Rath <nikolaus@rath.org>
Cc: Miklos Szeredi <miklos@szeredi.hu>, Max Reitz <mreitz@redhat.com>,
	Vivek Goyal <vgoyal@redhat.com>,
	Linux FS Devel <linux-fsdevel@vger.kernel.org>
Subject: Re: [PATCH] fuse: fix illegal access to inode with reused nodeid
Date: Wed, 16 Jun 2021 21:25:09 +0300	[thread overview]
Message-ID: <CAOQ4uxgzpKRWU2fFgF4OxROHZ84VZw7Ljqt5RvAi4b3ViTNfYg@mail.gmail.com> (raw)
In-Reply-To: <d7a38600-5b4b-487e-9362-790a7b5dde05@www.fastmail.com>

On Wed, Jun 16, 2021 at 8:25 PM Nikolaus Rath <nikolaus@rath.org> wrote:
>
> Hi Amir,
>
> On Wed, 16 Jun 2021, at 16:03, Amir Goldstein wrote:
> > Per request from Nikolaus, I modified the passthrough_hp example
> > to reuse inodes on last close+unlink, so it now hits the failure in the
> > new test with upstream kernel and it passes the test with this kernel fix.
> >
> > Thanks,
> > Amir.
> >
> > [2] https://github.com/libfuse/libfuse/pull/612
>
> Actually, I am no longer sure this was a good idea. Having the libfuse test suite detect problems that with the kernel doesn't seem to helpful.. I think the testsuite should identify problems in libfuse.  Currently, having the tests means that users might be hesitant to update to the newer libfuse because of the failing test - when in fact there is nothing wrong with libfuse at all.
>

I suppose you are right.
I could take the tesy_syscalls test to xfstest, but fuse support for
xfstests is still WIP.

> I assume the test will start failing on some future kernel (which is why it passed CL), and then start passing again for some kernel after that?

I was not aware that it passes CI.
There are no test results available on github.
I am not aware of any specific kernel version where the test should pass,
but the results also depend on the underlying filesystem.
If your underlying filesystem is btrfs, it does not reuse inode numbers
at all, so the test will not fail.

For me the test fails on ext4 and xfs on LTS kernel 5.10.
As I wrote in PR:
"...Fails the modified test_syscalls in this PR on upstream kernel"

If you revert the last commit the test would pass on upstream kernel:
80f2b8b ("passthrough_hp: excercise reusing inode numbers")

We could make behavior of passthrough_hp example depend
on some minimal kernel protocol version or new kernel capability like
FUSE_SETXATTR_EXT if Miklos intends to merge the fix for the coming
kernel release or we could just make that new test optional via pytest option.

After all, regardless of the kernel bug, this adds test coverage that was
missing, so it also covers a possible future regression in libfuse.

Let me know if you want me to implement any of the listed options.

Thanks,
Amir.