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
Setattr implementation was mixing up some flags, and missing some of
them.
Test: atest android.appsecurity.cts.ExternalStorageHostTest
Bug: 202785178
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: Id41fa30881766faad5858b658f5b6871c0ae46b3
Fixes: ANDROID: fuse-bpf: Fix perms on readdir
Test: Builds with and without CONFIG_FUSE_BPF
Bug: 202785178
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: If8b4603dd6f4bd159bfd68fc61c377dcb62ebcd0
Add checks for both fuse accesses and backing fs accesses
Bug: 202785178
Test: fuse_test passes, also atest ScopedStorageDeviceTest passes
Change-Id: Ida7d90e14ca36588a8cc19453e0d40b4f6f41aa9
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Uses lookup_one_len if given dentry is still negative/unlocked after
mkdir is called
Bug: 202785178
Test: atest android.scopedstorage.cts.device.ScopedStorageDeviceTest#testCreateAndDeleteEmptyDir
Change-Id: Id5c7dfd303c242d6966fab82d96712d289676857
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Bug: 202785178
Test: fuse_test runs, no leak on Android
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: If3c8dbe680f21c646e98f66140b842869c2c5abf
This will simply pass all mapping operations to the backing file if it
exists. This is sufficient for our needs in Android13, but must be
extended in the future.
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Bug: 202785178
Test: fuse_test passes
Change-Id: I3c51ef62415633ff0db039f25bfed1adf14e1e80
Bug: 202785178
Test: test_fuse passes on linux, feature works on cuttlefish
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: I987684b799b07391ccde350e98fde7976f5601aa
Contains squash of this commit:
Author: Nathan Chancellor <nathan@kernel.org>
Date: Fri Jan 20 09:40:12 2023 -0700
ANDROID: fuse: Restore upstream type of bitfields in fuse_args
Commit 88b7179fcdb59 ("ANDROID: fuse: Move functions in preparation for
fuse-bpf") changed the type of these bitfields from the upstream type of
'bool' to 'int', which causes several warnings with recent versions of
clang:
/builds/linux/fs/fuse/dir.c:168:19: error: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Werror,-Wsingle-bi
t-bitfield-constant-conversion]
args->out_argvar = true;
^ ~~~~
/builds/linux/fs/fuse/dir.c:492:18: error: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Werror,-Wsingle-bi
t-bitfield-constant-conversion]
args.out_argvar = 1;
^ ~
/builds/linux/fs/fuse/dir.c:1649:20: error: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Werror,-Wsingle-b
it-bitfield-constant-conversion]
ap.args.out_pages = true;
^ ~~~~
/builds/linux/fs/fuse/dir.c:1650:21: error: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Werror,-Wsingle-b
it-bitfield-constant-conversion]
ap.args.out_argvar = true;
^ ~~~~
/builds/linux/fs/fuse/dir.c:1651:23: error: implicit truncation from 'int' to a one-bit wide bit-field changes value from 1 to -1 [-Werror,-Wsingle-b
it-bitfield-constant-conversion]
ap.args.page_zeroing = true;
^ ~~~~
5 errors generated.
When fuse_args was moved back to the internal implementation in commit
9a5023967b4d2 ("ANDROID: fuse-bpf: Use fuse_bpf_args in uapi"), the type
was not restored. Do so now to fix the warnings and reduce the delta
with upstream.
Bug: 265200230
Change-Id: I4d51f331d842a1faff9a937140f0275130e70d73
Signed-off-by: Nathan Chancellor <nathan@kernel.org>
Bug: 202785178
Test: test_fuse passes on linux, feature works on cuttlefish
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Signed-off-by: Daniel Rosenberg <drosen@google.com>
Change-Id: Ie738893a821d1f5f252c4c6e86274d55a6f09965
commit 15db16837a35d8007cb8563358787412213db25e upstream.
Server responds to LOOKUP and other ops (READDIRPLUS/CREATE/MKNOD/...)
with ourarg containing nodeid and generation.
If a fuse inode is found in inode cache with the same nodeid but different
generation, the existing fuse inode should be unhashed and marked "bad" and
a new inode with the new generation should be hashed instead.
This can happen, for example, with passhrough fuse filesystem that returns
the real filesystem ino/generation on lookup and where real inode numbers
can get recycled due to real files being unlinked not via the fuse
passthrough filesystem.
With current code, this situation will not be detected and an old fuse
dentry that used to point to an older generation real inode, can be used to
access a completely new inode, which should be accessed only via the new
dentry.
Note that because the FORGET message carries the nodeid w/o generation, the
server should wait to get FORGET counts for the nlookup counts of the old
and reused inodes combined, before it can free the resources associated to
that nodeid.
Stable backport notes:
* This is not a regression. The bug has been in fuse forever, but only
a certain class of low level fuse filesystems can trigger this bug
* Because there is no way to check if this fix is applied in runtime,
libfuse test_examples.py tests this fix with hardcoded check for
kernel version >= 5.14
* After backport to stable kernel(s), the libfuse test can be updated
to also check minimal stable kernel version(s)
* Depends on "fuse: fix bad inode" which is already applied to stable
kernels v5.4.y and v5.10.y
* Required backporting helper inode_wrong_type()
Change-Id: I5f92158bc8ccdb50627fb035bb28d3ade51820b3
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Miklos Szeredi <mszeredi@redhat.com>
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/linux-fsdevel/CAOQ4uxi8DymG=JO_sAU+wS8akFdzh+PuXwW3Ebgahd2Nwnh7zA@mail.gmail.com/
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
commit 6e3e2c4362e41a2f18e3f7a5ad81bd2f49a47b85 upstream.
inode_wrong_type(inode, mode) returns true if setting inode->i_mode
to given value would've changed the inode type. We have enough of
those checks open-coded to make a helper worthwhile.
Change-Id: I346cb7a2d1cec2ca16096962d8adc9d7f848fc94
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
Signed-off-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
