Syzbot recently found a number of issues related to incremental-fs
(see bug numbers below). All have to do with the fact that incr-fs
allows mounts of the same source and target multiple times.
This is a design decision and the user space component "Data Loader"
expects this to work for app re-install use case.
The mounting depth needs to be controlled, however, and only allowed
to be two levels deep. In case of more than two mount attempts the
driver needs to return an error.
In case of the issues listed below the common pattern is that the
reproducer calls:
mount("./file0", "./file0", "incremental-fs", 0, NULL)
many times and then invokes a file operation like chmod, setxattr,
or open on the ./file0. This causes a recursive call for all the
mounted instances, which eventually causes a stack overflow and
a kernel crash:
BUG: stack guard page was hit at ffffc90000c0fff8
kernel stack overflow (double-fault): 0000 [#1] PREEMPT SMP KASAN
This change also cleans up the mount error path to properly clean
allocated resources and call deactivate_locked_super(), which
causes the incfs_kill_sb() to be called, where the sb is freed.
Bug: 211066171
Bug: 213140206
Bug: 213215835
Bug: 211914587
Bug: 211213635
Bug: 213137376
Bug: 211161296
Signed-off-by: Tadeusz Struk <tadeusz.struk@linaro.org>
Change-Id: I08d9b545a2715423296bf4beb67bdbbed78d1be1
The old and new mount user name spaces need to be populated
before calling vfs_rename(). Otherwise vfs_rename will try
to dereference a null ptr and segfault.
Bug: 211066171
Signed-off-by: Tadeusz Struk <tadeusz.struk@linaro.org>
Change-Id: I3656073581218107fc3b1a52ebe7bcfd81a10fc2
This reverts three increment-fs commits:
d5faa13b5910412e10c67ad88c9349
This is to fix the incrementalinstall test.
Can now install the same apk twice, and repeated installs are stable.
Bug: 217661925
Bug: 219731048
Signed-off-by: Tadeusz Struk <tadeusz.struk@linaro.org>
Change-Id: Ia8488d728218881ed17e4d68cab21b0b152e3ca4
The old and new mount user name spaces need to be populated
before calling vfs_rename().
Bug: 211066171
Signed-off-by: Tadeusz Struk <tadeusz.struk@linaro.org>
Change-Id: Ieac6975abb4131c8f5bdefe25b5f241c80023e38
Cleanup incremental-fs left overs on umount, otherwise incr-fs will
complain as below:
BUG: Dentry {i=47a,n=.incomplete} still in use [unmount of incremental-fs]
This requires vfs_rmdir() of the special index and incomplete dirs.
Also free options.sysfs_name in incfs_mount_fs() instead of in
incfs_free_mount_info() to make it consistent with incfs_remount_fs().
Since set_anon_super() was used in incfs_mount_fs() the incfs_kill_sb()
should use kill_anon_super() instead of generic_shutdown_super()
otherwise it will leak the pseudo dev_t that set_anon_super() allocates.
Bug: 211066171
Signed-off-by: Tadeusz Struk <tadeusz.struk@linaro.org>
Change-Id: I7ea54db63513fc130e1997cbf79121015ee12405
Syzbot recently found a number of issues related to incremental-fs
(see bug numbers below). All have to do with the fact that incr-fs
allows mounts of the same source and target multiple times.
The correct behavior for a file system is to allow only one such
mount, and then every subsequent attempt should fail with a -EBUSY
error code. In case of the issues listed below the common pattern
is that the reproducer calls:
mount("./file0", "./file0", "incremental-fs", 0, NULL)
many times and then invokes a file operation like chmod, setxattr,
or open on the ./file0. This causes a recursive call for all the
mounted instances, which eventually causes a stack overflow and
a kernel crash:
BUG: stack guard page was hit at ffffc90000c0fff8
kernel stack overflow (double-fault): 0000 [#1] PREEMPT SMP KASAN
The reason why many mounts with the same source and target are
possible is because the incfs_mount_fs() as it is allocates a new
super_block for every call, regardless of whether a given mount already
exists or not. This happens every time the sget() function is called
with a test param equal to NULL.
The correct behavior for an FS mount implementation is to call
appropriate mount vfs call for it's type, i.e. mount_bdev() for
a block device backed FS, mount_single() for a pseudo file system,
like sysfs that is mounted in a single, well know location, or
mount_nodev() for other special purpose FS like overlayfs.
In case of incremental-fs the open coded mount logic doesn't check
for abusive mount attempts such as overlays.
To fix this issue the logic needs to be changed to pass a proper
test function to sget() call, which then checks if a super_block
for a mount instance has already been allocated and also allows
the VFS to properly verify invalid mount attempts.
Bug: 211066171
Bug: 213140206
Bug: 213215835
Bug: 211914587
Bug: 211213635
Bug: 213137376
Bug: 211161296
Signed-off-by: Tadeusz Struk <tadeusz.struk@linaro.org>
Change-Id: I66cfc3f1b5aaffb32b0845b2dad3ff26fe952e27
Add a flag option to get xattr method that could have a bit flag of
XATTR_NOSECURITY passed to it. XATTR_NOSECURITY is generally then
set in the __vfs_getxattr path when called by security
infrastructure.
This handles the case of a union filesystem driver that is being
requested by the security layer to report back the xattr data.
For the use case where access is to be blocked by the security layer.
The path then could be security(dentry) ->
__vfs_getxattr(dentry...XATTR_NOSECURITY) ->
handler->get(dentry...XATTR_NOSECURITY) ->
__vfs_getxattr(lower_dentry...XATTR_NOSECURITY) ->
lower_handler->get(lower_dentry...XATTR_NOSECURITY)
which would report back through the chain data and success as
expected, the logging security layer at the top would have the
data to determine the access permissions and report back the target
context that was blocked.
Without the get handler flag, the path on a union filesystem would be
the errant security(dentry) -> __vfs_getxattr(dentry) ->
handler->get(dentry) -> vfs_getxattr(lower_dentry) -> nested ->
security(lower_dentry, log off) -> lower_handler->get(lower_dentry)
which would report back through the chain no data, and -EACCES.
For selinux for both cases, this would translate to a correctly
determined blocked access. In the first case with this change a correct avc
log would be reported, in the second legacy case an incorrect avc log
would be reported against an uninitialized u:object_r:unlabeled:s0
context making the logs cosmetically useless for audit2allow.
This patch series is inert and is the wide-spread addition of the
flags option for xattr functions, and a replacement of __vfs_getxattr
with __vfs_getxattr(...XATTR_NOSECURITY).
Bug: 204981027
Link: https://lore.kernel.org/lkml/20211117015806.2192263-2-dvander@google.com
Change-Id: Id2c6fa6eeb2b5cca5a11e0cd02a3fbf2a5fcbef4
Signed-off-by: David Anderson <dvander@google.com>
Signed-off-by: Mark Salyzyn <salyzyn@android.com>
In commit edb0872f44 ("block: move the bdi from the request_queue to
the gendisk"), the backing device info moved .h files which breaks the
building of incfs. Fix this up like was done for fat, just include the
correct .h file.
Fixes: edb0872f44 ("block: move the bdi from the request_queue to the gendisk")
CC: Paul Lawrence <paullawrence@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Change-Id: Iccc990b1b01af4580192921410b915b91dc6a155
<linux/blkdev.h> no longer includes <linux/pagemap.h>.
Besides, source files should explicitly include headers providing used
functionality anyway.
Fixes: 4ee60ec156 ("include: remove pagemap.h from blkdev.h")
Signed-off-by: Lee Jones <lee.jones@linaro.org>
Change-Id: I6d28e5cc100780ecd74f5d772f08fd4b0f977898
Prior change
ANDROID: Incremental fs: stat should return actual used blocks
adds blocks to getattr. Unfortunately the code always looks for the
backing file, and pseudo files don't have backing files, so getattr
fails for pseudo files.
Bug: 186567511
Test: incfs_test passes, can do incremental installs on test device
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: Ia3df87f3683e095d05c822b69747515963c95f1c
Adding seven sysfs entries per mount:
reads_failed_timed_out
reads_failed_hash_verification
reads_failed_other
reads_delayed_pending
reads_delayed_pending_us
reads_delayed_min
reads_delayed_min_us
to allow for status monitoring from userland
Change-Id: I50677511c2af4778ba0c574bb80323f31425b4d0
Test: incfs_test passes
Bug: 160634343
Bug: 184291759
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Lots of different vfs changes happened in 7d6beb71da ("Merge tag
'idmapped-mounts-v5.12' of
git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux") which were
not made to incfs as it is out-of-tree :(
So fix it up by doing a quick port to the initial user namespace and
hopefully all works properly now, at least it builds :)
Fixes: 7d6beb71da ("Merge tag 'idmapped-mounts-v5.12' of git://git.kernel.org/pub/scm/linux/kernel/git/brauner/linux")
Cc: Paul Lawrence <paullawrence@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Change-Id: I32899a792fbfad9455d504868e7a02d30a0a6049
This reverts commit 3484eba91d.
It's causing too many merge issues, so revert it for now and hopefully
bring it back later...
Cc: Mark Salyzyn <salyzyn@android.com>
Bug: 133515582
Bug: 136124883
Bug: 129319403
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Change-Id: Id70a37d9471eff58f0027a1371538858136604d3
Add ioctl to return the verity file digest, compatible with the identical
ioctl in fs/verity/.
Bug: 160634504
Test: incfs_test passes
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: I1bc2dc975b9be122e1c831a25a1d44f27a360f3c
Now fsverity state is preserved across inode eviction.
Added incfs.verity xattr to track when a file is fs-verity enabled.
Bug: 160634504
Test: incfs_test passes
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: I41d90abd55527884d9eff642c9834ad837ff6918
Add FS_IOC_GETFLAGS ioctl to incfs. Currently this will only get the
S_VERITY flag.
Bug: 160634504
Test: incfs_test passes
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: Id79add0db0d66f604ca0f222fe5faec91450ade5
Add FS_IOC_ENABLE_VERITY ioctl
When called, calculate measurement, validate signature against fsverity,
and set S_VERITY flag.
This does not (yet) preserve the verity status once the inode is
evicted.
Bug: 160634504
Test: incfs_test passes
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: I88af2721f650098accc72a64528c7d85b753c7f6
Bug: 177075428
Test: incfs_test passes
atest GtsIncrementalInstallTestCases has only 8 failures
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: I73accfc1982aec1cd7947996c25a23e4a97cfdac
.blocks_writen file handling was missing some operations:
SELinux xattr handlers, safety checks for it being a
pseudo file etc.
This CL generalizes pseudo file handling so that all such
files work in a generic way and next time it should be
easier to add all operations at once.
Bug: 175823975
Test: incfs_tests pass
Change-Id: Id2b1936018c81c62c8ab4cdbaa8827e2679b513f
Signed-off-by: Yurii Zubrytskyi <zyy@google.com>
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Rmove bc_mutex used to protect metadata chain, now that is only
read at file open time
Remove certain unused mount options
Bug: 172482559
Test: incfs_test passes
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: Id70e5a5d08e5de79f391e19ea97e356f39a3ed51
report_uid was not being initialized, leading to random behavior
Bug: 172480517
Test: incfs_test passes
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: Ib121136d6f570f99e9060bdde9aa43ff2995514e
Also fixed two bugs in the process:
is_pseudo_filename was not previously checking for .log, so an attempt
to create a .log would succeed.
All ioctls could be called on all files. ioctls now set on the correct
files.
Bug: 162856396
Test: incfs_test passes
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: I3f1e87d018836f51a97897880dd70181db4f7169
It's still magic number issue which cannot be compatible with
arm-32 platform, although we try to fix it in Iae4f3877444
("ANDROID: Incremental fs: magic number compatible 32-bit"),
there is still incompatible scenario, such as: get_incfs_node(),
it will return NULL then kernel exception will be trigger because
of NULL pointer access. (inode_set() -> get_incfs_node(), then used
node->xxx directly)
We change magic number directly, otherwise, we must fix above issues one by one.
Bug: 159772865
Fixes: Iae4f3877444("ANDROID: Incremental fs: magic number compatible 32-bit")
Signed-off-by: Peng Zhou <Peng.Zhou@mediatek.com>
Signed-off-by: mtk81325 <peng.zhou@mediatek.com>
Change-Id: I71f279c1bb55ea296ab33a47644f30df4a9f60a6
Test: incfs_test on a virtio 9p drive. Note test 6 fails still,
but I think this is a result of caching directory entries over a
network file system.
Bug: 161802292
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: I6986fb3e9b403181cf81024046f394960caf4620
Use Read-Write locks for reading/writing segment in blockmap.
This should allow parallel reads when there are
multiple reads within same segment.
A small optimization in pending_reads_read(). Since
incfs_collect_pending_reads() already iterate to
populate buffer, new_max_sn - highest serial number
among all the pending read buffer can be done in the same
loop instead of looping again in pending_reads_read().
Bug: 161566104
Test: kernel selftest - incfs_test and incfs_perf
Signed-off-by: Akilesh Kailash <akailash@google.com>
Change-Id: Id00376b0e4cb8c0c0bc8264cdddd6f38c4aa85f0
1: Invoke kunmap(page) in error path
2: Validate NULL checks at few places in the code.
3: path_put() should not be invoked if path entry is null.
Although path_put() checks for NULL condition internally,
caller should gracefully handle it.
Bug: 161565969
Test: kernel selftest - incfs_test, incfs_perf
Signed-off-by: Akilesh Kailash <akailash@google.com>
Change-Id: Ie4dfaaba4b09f4798d492f8a25dd9dcc8da89e51
Incfs's magic is bigger than 32-bit, but super block structure's
s_magic is unsigned long which is 32-bit in ARM 32-bit platform.
Do the cast for magic!
Bug: 159772865
Signed-off-by: Peng Zhou <Peng.Zhou@mediatek.com>
Signed-off-by: mtk81325 <peng.zhou@mediatek.com>
Signed-off-by: Paul Lawrence <paullawrence@google.com>
Change-Id: Iae4f38774440c7d6ae44529d4f0f8ebb2ec5dacc
READ/WRITE_ONCE are for atomic data types, not for structures. Fix this
up by doing a memcpy to make it explicit just how messy this copy is...
This fixes a build error on 5.8-rc1, as things are more strict, odds are
it's also wrong in other kernel versions as well...
Cc: Daniel Mentz <danielmentz@google.com>
Cc: Paul Lawrence <paullawrence@google.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@google.com>
Change-Id: I7ecd3d05bd94c936dd5e69c63028458786f37a78
