commit a7f0d7aab0b4f3f0780b1f77356e2fe7202ac0cb upstream.
If an incoming FUSE request can't fit on the virtqueue, the request is
placed onto a workqueue so a worker can try to resubmit it later where
there will (hopefully) be space for it next time.
This is fine for requests that aren't larger than a virtqueue's maximum
capacity. However, if a request's size exceeds the maximum capacity of the
virtqueue (even if the virtqueue is empty), it will be doomed to a life of
being placed on the workqueue, removed, discovered it won't fit, and placed
on the workqueue yet again.
Furthermore, from section 2.6.5.3.1 (Driver Requirements: Indirect
Descriptors) of the virtio spec:
"A driver MUST NOT create a descriptor chain longer than the Queue
Size of the device."
To fix this, limit the number of pages FUSE will use for an overall
request. This way, each request can realistically fit on the virtqueue
when it is decomposed into a scattergather list and avoid violating section
2.6.5.3.1 of the virtio spec.
Change-Id: I78a1abfe6cb5484ec0f59c3aa841f119b6a5ff16
Signed-off-by: Connor Kuehl <ckuehl@redhat.com>
Reviewed-by: Vivek Goyal <vgoyal@redhat.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Yang Bo <yb203166@antfin.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
To avoid a BUG_ON, we must disable migration before running any bpf
program.
Bug: 273620140
Test: fuse-test passes, no bug with CONFIG_DEBUG_ATOMIC_SLEEP
Change-Id: I9ed69ae93fc4b922782fccde293cb02b3eae3f06
Signed-off-by: Paul Lawrence <paullawrence@google.com>
(cherry picked from commit 0411f8d9dfa88e441456b5bcf684c5ec28ef08a7)
Backing files were sometimes put twice before, this fixes it so backing
files sent in response to lookups are closed exactly once always
Test: fuse_test pases, Android no longer throws a double close
Bug: 273737310
Change-Id: Ifa75ffd846185cfabfd1f5bad504078d955c99ed
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Signed-off-by: Alistair Delva <adelva@google.com>
If a lookup finds an existing inode, it must not change the existing bpf
program since it may be in use.
Bug: 267095363
Test: fuse_test, atest CtsScopedStorageHostTest
Change-Id: Icb00681fbcd51fdd4b0764906509093d98caeec4
Signed-off-by: Paul Lawrence <paullawrence@google.com>
commit df8629af293493757beccac2d3168fe5a315636e upstream.
Failure to do so may result in EEXIST even if the file only exists in the
cache and not in the filesystem.
The atomic nature of O_EXCL mandates that the cached state should be
ignored and existence verified anew.
Reported-by: Ken Schalk <kschalk@nvidia.com>
Change-Id: I7bc67048363004f94dfe7b74d0873c019f06eaef
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Wu Bo <bo.wu@vivo.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Note that this is specific for the non-upstreamed version
Bug: 202785178
Test: cat /sys/fs/fuse/fuse_bpf_major_version
Change-Id: I68f9ca56778874975428839dfc1fd8f48b11bd75
Signed-off-by: Paul Lawrence <paullawrence@google.com>
[ Upstream commit 9fa248c65bdbf5af0a2f74dd38575acfc8dfd2bf ]
There's a race in fuse's readdir cache that can result in an uninitilized
page being read. The page lock is supposed to prevent this from happening
but in the following case it doesn't:
Two fuse_add_dirent_to_cache() start out and get the same parameters
(size=0,offset=0). One of them wins the race to create and lock the page,
after which it fills in data, sets rdc.size and unlocks the page.
In the meantime the page gets evicted from the cache before the other
instance gets to run. That one also creates the page, but finds the
size to be mismatched, bails out and leaves the uninitialized page in the
cache.
Fix by marking a filled page uptodate and ignoring non-uptodate pages.
Reported-by: Frank Sorenson <fsorenso@redhat.com>
Fixes: 5d7bc7e868 ("fuse: allow using readdir cache")
Cc: <stable@vger.kernel.org> # v4.20
Change-Id: I1dea0531dd4d2cb0b89b8e1ee6796339fc2dc624
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
Test: fuse_test passes, with appropriate user code directories can be
hidden
Bug: 219958836
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: Ia77d4bf9e63d0dd4535e53e9e07c3216fd4b8430
kernelci reported the following error when !CONFIG_FUSE_BPF:
fs/fuse/dir.c:269:40: error: no member named 'backing_inode' in 'struct fuse_inode'
Fixed by wrapping that new code in #ifdef CONFIG_FUSE_BPF.
Fixes: 094905c87771 ("ANDROID: fuse-bpf: Always call revalidate for
backing")
Signed-off-by: Todd Kjos <tkjos@google.com>
Change-Id: I08760f340cebf21d3084ca84ac1792bc70f311ff
094905c87771 ("ANDROID: fuse-bpf: Always call revalidate for backing")
called dget_parent/dput improperly within an RCU context. Additionally,
it failed to free/put some references.
Fixes: 094905c87771 ("ANDROID: fuse-bpf: Always call revalidate for backing")
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: Iedeee7550ff88366bc5310eedece285019336814
If we have a backing dentry, we must call it's revalidate always, or we
may end up using an invalid lower dentry. Revalidate is called for
dentries, not inodes, and the dentry may be negative. This goes against
all of the macro conventions, so we're just calling the backing function
directly.
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Bug: 219958836
Test: fuse_test
Change-Id: Ia28da5bd2ce42b40466c577137d5440d9f2f6600
Cleaned up some args, and adjusted so we can choose to not update the
actual values for the next patch.
Test: fuse_test
Bug: 219958836
Change-Id: I025b7026357b119e2cd588f25f0976f5d4b68090
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Currently we have 2 different problems
1. Every revalidate considered as a error because of added
args->out_argvar = true; inside fuse_lookup_init which makes
fuse_simple_request return out argument size which is
considered as an error by revalidate code.
2. We’re ignoring backing_fd and bpf_program set by daemon
lookup code called by revalidate.
Problem 1 makes any revalidate (lookup to userspace) useless and any result
lead us to the full lookup because it was interpreted as an error.
This CL fixes both and introducing revalidate test case which makes sure:
1. We’re receiving only one lookup as a part of revalidate
2. We’re setting backing_fd as a part of revalidate’s lookup result
Test is failed before the fix and passed after.
Bug: 219958836
Test: Booted device 5 times to make sure we’re not receiving redundant
lookups anymore.
Test: selftests
Signed-off-by: Dmitrii Merkurev <dimorinny@google.com>
Change-Id: Ifa62e56b42ca5580b25682eb5f16b5c91826cf49
If we have a nodeid of 0, we've probably got a backing inode, and a
regular getattr will be fast. Otherwise, userspace is likely ill suited
to properly handle a readdirplus anyways.
Test: fuse_test
Bug: 219958836
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: I02f031d87dcc5fcbe1e080e4f8ec92187b00fe2d
get_fuse_inode uses container_of, which results in a strange result if
the inode is NULL. We should check if the inode is NULL instead.
Fixes: 4ad093cae178 ("ANDROID: fuse-bpf: Make inodes with backing_fd reachable")
Bug: 219958836
Change-Id: I386c4641edaa4dbc8d8e02f592c959c206851eda
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Usage of uninitialized boolean is potentially can cause annoying
and “hard to catch” types of problems. Currently we have 1
case where we use uninitialized boolean:
int fuse_readdir(struct file *file, struct dir_context *ctx)
And I constantly see that every userspace readdir operation
causes an infinite cycle inside the Kernel for my QEMU tests
(gcc).
This problem isn’t reproducible inside cuttlefish, probably
because we use clang toolchain.
Bug: 219958836
Test: atest ScopedStorageDeviceTest
Test: selftests
Change-Id: I2c38056448cd2910e0cb20da5839d7db9ebd26b9
Signed-off-by: Dmitrii Merkurev <dimorinny@google.com>
for regular FUSE fuse_iget
Currently, when we’re trying to find inode based on their
backing inode we strictly checking on nodeid == 0, so
basically we’re not supporting nodeid != 0 for inode,
which is backed by another one. Alongside with this, we’re
using backing_inode as a hash for inode which make this inode
not reachable for regular FUSE fuse_iget that as a result
causing backing_inode losing because instead of getting
existent one (with backing inode) we create a new one as
a part of readdirplus.
For more details please check: go/fuse-loosing-inode-with-backing
Bug: 219958836
Test: Manually checked that /data and /obb inodes
always have inode numbers configured.
Co-developed-by: Paul Lawrence <paullawrence@google.com>
Change-Id: If6a5fb340561ac6320d3c4e86215f1bcd4c2c10c
Signed-off-by: Dmitrii Merkurev <dimorinny@google.com>
to execute backing revalidate"
This reverts commit b610eff230f2ce92fd48502d71ad0993792f73d3.
Reason for revert: I broke BPF calling logic with this one. Possible
fix is here:
https://android-review.googlesource.com/c/kernel/common/+/2132134 but
we're still discussing possible way to go there:
go/fuse-bpf-revalidate-problem
Change-Id: I517941a2c341999dc8133b93cf045ec67bcf8a9e
Signed-off-by: Dmitrii Merkurev <dimorinny@google.com>
Usually as a result of initial fuse lookup with bpf enabled we have following dentry:
-----------------------------------------------------------------
| dentry /storage/emulated/0/Android/data |
| inode |
| backing_inode: /pass_through/emulated/0/Android/data |
-----------------------------------------------------------------
Every communication with this folder will have to go
through fuse_dentry_revalidate(dentry, flags) which can move forward by:
1. If the timeout is not reached, just ignore it
2. If entry has backing_inode and bpf is not against it, execute revalidate on backing FS (inside kernel)
3. Move to userspace to revalidate
But for some reason currently, we're checking parent inode (not one that we wanna revalidate) to have
backing inode that we can use to execute operations on. Basically, the whole flow looks like this:
1. Receiving revalidate event for fuse_dentry_revalidate(/storage/emulated/0/Android/data, flags)
2. Checking .../0/Android/ inode to have backing inode <------------------------ Primary problem is HERE
3. Moving to the userspace with pf_lookup(/storage/emulated/0/Android, data)
4. Even though successfully handled lookup on the fuse daemon side, kernel cannot interpret
the result due to fuse_simple_request and fuse_lookup_init logic changes <------- Secondary problem is HERE
5. Because of the problems I mentioned before, full lookup is triggered on the kernel side so we receive the second pf_lookup to the userspace
Fixing primary problem by executing backing revalidate on the current inode (not the parent one).
Bug: 234346312
Test: Manually made sure don't have any userspace calls for interactions inside directory with backing one.
Test: Manually check youtube app is successfully saving exo cache into the external storage cache folder.
Test: atest --test-mapping packages/providers/MediaProvider
Signed-off-by: Dmitrii Merkurev <dimorinny@google.com>
Change-Id: Id57f1944302076d93ebef255533dfc53e8c30f20
Added #ifdefs around fuse-bpf init/cleanup code
Bug: 202785178
Test: builds with and without CONFIG_FUSE_BPF
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: Ie15bb04e439b496e4842303437b3f55c3da14f2c
fuse_args is not suitable for use in the uapi - it is not stable, and
contains internal pointers. Replace with stable equivalent.
The end_offset values are currently unused and unset, but will be used
in a follow up patch by the verifier.
Test: fuse_test, atest ScopedStorageDeviceTest pass
Bug: 202785178
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: Ic1c12f9706aeae233cc30a0d68ed2533030e485b
We had a size mismatch for the return value, leading to EIOCBQUEUED
getting interpreted as a return size instead of an error code.
Test: generic/467, generic/013, and fuse_test
Bug: 217570523
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: I64f9d5263f8b37d3c0e286467f9351997b294cc2
Allocates the iocb we create for asynchronous IO from a cache instead of
a regular kzalloc
Test: generic/467 and fuse_test
Bug: 217570523
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: I27dcec89cd585835f6a8e80e1ae30c503f4038c8
The current name is a bit confusing. iocb_fuse could refer to the iocb
passed to fuse or created by fuse. The new name unambiguously refers to
the one passed in to fuse.
Test: compiles, behavior unchanged
Bug: 217570523
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: I955500eb8a3186252427fd06ca6e99b4fec469b6
Existing fixattr was adjusting the same node twice.
Bug: 226655982
Test: generic/241 generic/269
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: I4b1cb6d626ee6bd9010012ac126b78f14d6157d0
Fuse uses generic_file_llseek, so we must account for that in readdir to
ensure we read from the correct offset in the lower filesystem.
Bug: 226655281
Test: generic/257, fuse_test
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: Ie752c1c645e95b7c03ef9497562758a5c42b514a
Properly handle the async case. The existing bpf operations will likely
need to be reworked. Given that they don't allow altering anything as
is, this change just incrementally moves us in the right direction.
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Test: generic/467 and fuse_test
Bug: 217570523
Change-Id: I31c0b48bf3d674efecad4bff4ea8b482c4e7da45
This doesn't cause any behavior changes and will be used by overlay async
IO implementation.
Remove 'kiocb_clone' from passthrough.c as this is the proper upstream
commit which adds the function.
Change-Id: Ic3d0ebd5cc2d945358f7295697ae02a829e2e919
Signed-off-by: Jiufei Xue <jiufei.xue@linux.alibaba.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Adds support for lseek via fuse-bpf
Bug: 224855060
Test: bpf_test_lseek
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: Ic282940d53b9bb44a291cb3a5dfe09847b4e5c9a
Ensure finalize has access to the returned error code, if it had one.
Test: generic/377
Bug: 217570523
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: Id9a0aca10c3158f8b414e0cbc2dd1282bb5ef16b
This moves the backing/fd files to their own space, instead of reusing
the userspace provided fds.
Bug: 222619123
Test: fuse_test passes, on cuttlefish CtsCameraTestCases passes
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: I5d3b1ea8299f249ef5adc1ce2b7f45404a041208
Bug: 222497969
Test: idle kingdom launches sucessfully with no mediaprovider crashes
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: Ia5f842fd4a0bf9a21d7c88874b13b84d42ab2c4d
Bug: 222619123
Test: fuse_test passes, on cuttlefish CtsCameraTestCases passes
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: I54c148206b5ad5ae5737939bcb076cbe6c40129c
If the lower filesystem has deleted a file or folder behind our back, we
should drop it as well.
Bug: 221093504
Test: atest android.hardware.cts.CameraTest#testJpegThumbnailSize
several times in a row
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: Ibd92dea83a82dd7ab21269ae5d2533e4826f5fb7
In the backing case, we initially want to call out to the fuse-bpf
implementation, and only fall back to userspace if that requests it.
Otherwise we end up making requests to userspace that the daemon may not
be equiped to respond to.
Change-Id: If3780aa8b7c45558717a9efba0b1781e8d63a3c0
Bug: 217570523
Test: generic/099
Signed-off-by: Daniel Rosenberg <drosen@google.com>
This is useful for testing fuse-bpf directly on a backing folder.
Bug: 217570523
Test: mount -t fuse [DEVNAME] [mntpoint] -o user_id=0,group_id=0,rootmode=0040000,
no_daemon,root_dir=[backingfd]
Change-Id: I9ac13c3f707d71cbb74dba10eda5778bf3e83233
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Adds support for FUSE_STATFS, needed to run various filesystem tests
Bug: 217570523
Test: bpf_test_statfs
Change-Id: I5ee13e880118c5c79c4ca17bb2e902a3e17a7eb8
Signed-off-by: Daniel Rosenberg <drosen@google.com>
filldir used strcpy, potentially leading to writing the ending null past
the current page. fuse_dirents are not null terminated, so we switch to
using memcpy to avoid adding an extraneous null, which would be
overwritten if the name was already byte aligned.
Bug: 217570523
Test: generic/027
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: Ic5d1f1887a113e1a3319998bad47cfbac3d90baa
If we hit an error during fuse_create_open, some variables will be
undefined during the finalize, so check that they were actually
initialized before accessing.
Bug: 217570523
Test: attempt to over fill disk
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: I094564b83e49eec2a6bac5bd050b4f7327b0c979
Maps would crash every second launch. This was caused by maps receiving
an incorrect file size after truncate, then mapping the file.
Bug: 215486645
Test: fuse_test + launch maps 100 times & look for native crashes
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: I13b0211330fb48592864ab53f8ffff60c19aab11
We should only apply the mask ourselves if we're not
using POSIX acls
Bug: 215212818
Test: com.android.cts.externalstorageapp.CommonExternalStorageTest
#testAllPackageDirsWritable, verify files and cache permissions
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: If105afe62a60b93cbce1ca5ab5caf11f008aa7db
In a move operation, if the FROM file has a backing inode associated and
is handled in backing, it might happen that the TO file does not have a
backing path associated (yet), maybe because the FUSE daemon didn't have
the chance to traverse it.
Thus this special case would mistakenly trigger EBADF, while EXDEV is
more appropriate.
Bug: 202785178
Test: mv /storage/emulated/0/Android/data/<pkg>/file /sdcard/DCIM
Signed-off-by: Alessio Balsini <balsini@google.com>
Change-Id: I513c0e17c128ed9181a6b96fbf9f0b950e78be77
