Commit graph

42 commits

Author SHA1 Message Date
hyeeun.jun@samsung.com
49545ab897 Fix Deadlock Issue On AppFuseBridge
There are two locks used by AppFuseBridge.
First is it's object lock, and the second is a mutex lock in app fuse library.
There are two oppsite routines to get those locks.

  (Thread A) Got Java lock -> Try to get Native lock
  (Thread B)        Got Native lock -> Try to get Java lock

Bug : https://issuetracker.google.com/issues/145707568
Signed-off-by: hyeeun.jun@samsung.com <hyeeun.jun@samsung.com>

The order must be followed to obtain two locks.
If not, the dead lock will be caused.
Therefore we change the routine to get the mutex lock first, and the object lock later.
2020-02-18 04:33:06 +00:00
Anton Hansson
b6d9fbddc6 Merge "Close /dev/fuse FD before calling onClosed" into qt-dev 2019-05-31 15:59:39 +00:00
Anton Hansson
c9e873f274 Close /dev/fuse FD before calling onClosed
This works around a deadlock when a bridge that is about to be closed
is reused for a new call to openFile. The call to open() ends up holding
the vold lock, waiting for appfuse to respond. The appfuse event loop
calls onClosed(), which ends up calling vold.unmountAppFuse(), which
cannot get the lock.

Closing this file descriptor causes any current calls to open() on its
mount path to fail with either ECONNABORTED or ENOTCONN, allowing the
event loop to make progress, call onClosed() and unmount the path.

Note that the failed call to open() will result in a retry, which
will create a new appfuse bridge. This is not ideal but not a new
problem -- the common case here is that that each call to
openProxyFileDescriptor creates a new bridge. This should ideally
be improved.

Bug: 132344997
Test: flick through info of photos with location info attached
Exempt-From-Owner-Approval: verbal approval of approach
Change-Id: I878e5cf86f18c5233f8505f52eb9db076bd72d01
2019-05-31 15:58:45 +00:00
Daichi Hirono
ca0d4ffbe3 Fix BridgeEpollController to handles EAGAIN correctly
When reading/writing proxy FD, if it returns EAGAIN,
BridgeEpollController updates epoll entries to observe specific FD
events. Before updating epoll entries, BridgeEpollController checks if
it really needs to update by comparing |state_| and |last_state_|.
|last_state_| has not been updated correctly so it resulted in wrong
epoll settings and keeps blocking the event loop.

Bug: 134104939
Test: atest libappfuse_test
Change-Id: I1c4a0164c1c016baf24ecfd523476ced981d3b28
2019-05-31 12:51:05 +09:00
Nick Kralevich
f47c91053f use epoll_create1(EPOLL_CLOEXEC)
epoll_create(0) leaks file descriptors. Use epoll_create1(EPOLL_CLOEXEC)
instead.

Bug: 120983106
Test: compiles and boots
Change-Id: I2a733d4482d6a74ceb3254e501cdb5f6de0cd5dc
2018-12-17 09:32:23 -08:00
Ryo Hashimoto
3d95d88b14 Stop using SO_SNDBUFFORCE
Use SO_SNDBUF which doesn't require CAP_NET_ADMIN instead.
Change the value of kFuseMaxWrite to 128KB.

In the kernel code, there is a constant FUSE_MAX_PAGES_PER_REQ which
limits the size of requests to 128KB.

Bug: 74725300
Test: atest android.os.storage.cts.StorageManagerTest

Change-Id: Ic3a8f1a7378d027a6c0ee054cedc2c9f4b7509ad
2018-03-15 14:22:27 +09:00
Elliott Hughes
dc699a269f bpfmt.
Bug: N/A
Test: builds
Change-Id: I89ad00e1c4c7e0767bc80a7ac7935a4d55e090ac
2018-02-16 17:58:14 -08:00
Elliott Hughes
3289b9c928 Merge "Add OWNERS." 2017-12-07 23:21:26 +00:00
Elliott Hughes
693d63f9cf Add OWNERS.
Bug: N/A
Test: N/A
Change-Id: Ie785058c0f5eb9b4086c98ccba6e63e3ed411b65
2017-12-07 13:30:03 -08:00
Lennart Wieboldt
6da3de27be Merge "Remove LOCAL_CLANG and clang: true" am: 80ec81cf4b am: f7b315c985 am: 8a8b97b271
am: e1332dd01b

Change-Id: I193d86abf96e64b29efc7266f4fa3a26b5d2b3af
2017-07-25 22:39:58 +00:00
Lennart Wieboldt
e1332dd01b Merge "Remove LOCAL_CLANG and clang: true" am: 80ec81cf4b am: f7b315c985
am: 8a8b97b271

Change-Id: I7e691abe2e2ccec5d9477c528d603c081c0a5661
2017-07-25 22:35:56 +00:00
Lennart Wieboldt
f7b315c985 Merge "Remove LOCAL_CLANG and clang: true"
am: 80ec81cf4b

Change-Id: Ia7f79d8e25ee9870fe44d198568d0e5dabdff811
2017-07-25 22:28:56 +00:00
Lennart Wieboldt
cd15fc7ba8 Remove LOCAL_CLANG and clang: true
clang is the default compiler since Android nougat

Test: mma & verified it´s still build with clang
Change-Id: I34adaeef2f6558a09f26027271222bad94780507
Signed-off-by: Lennart Wieboldt <lennart.1997@gmx.de>
2017-07-25 14:29:50 +02:00
Daichi Hirono
926acb637f Remove ScopedLogSeverity for debugging
The ScopedLogSeverity was added to observe APCT failures, which turned
out to be a compiler optimization error.

Bug: 62429763
Test: None
Change-Id: Ibb45d018d8eaf4b29cb417da80ae5f0b000dda8e
2017-07-14 16:44:16 +09:00
Daichi Hirono
d9cda90ad4 Add volatile to temporary variable.
FuseBuffer::HandleNotImpl save the value of |request.header.unique| to the
temporary variable, clear the buffer which is a union of |request| and
|response|, then write back the unique value to response.header.unique.

Before the CL, the temporary variable was wrongly removed by the compiler
optimization, and response.header.unique was always 0. The CL adds
volatile modifier as workaround to prevent the compiler optimization
from removing the temporary value.

Bug: 62429763
Test: libappfuse_tests
Change-Id: Ia853f805633f646f316f585a35c7b018000b6eb3
(cherry picked from commit a6dee5e279)
2017-06-23 07:15:27 +00:00
Daichi Hirono
a6dee5e279 Add volatile to temporary variable.
FuseBuffer::HandleNotImpl save the value of |request.header.unique| to the
temporary variable, clear the buffer which is a union of |request| and
|response|, then write back the unique value to response.header.unique.

Before the CL, the temporary variable was wrongly removed by the compiler
optimization, and response.header.unique was always 0. The CL adds
volatile modifier as workaround to prevent the compiler optimization
from removing the temporary value.

Bug: 62429763
Test: libappfuse_tests
Change-Id: Ia853f805633f646f316f585a35c7b018000b6eb3
2017-06-23 11:23:12 +09:00
Daichi Hirono
afa3453256 Skip FUSE request from /dev/fuse if unique=0
APCT log shows that we got FUSE request unique=0 and replying to such
request causes a EINVAL.

The possible reasons of getting unique=0 here are:

 * /dev/fuse actually submits such requests. In this case, not replying
   to such request probabbly safe as the kernel cannot wait corresponding
   response without a unique number. We can observing the kernel code to
   find out what unique=0 actually means.
 * Memory corruption happens and unique number are cleared with zero.
   In this case, if we skip unique=0 request, libappfuse does not reply
   to the kernel request and APCT result will become timeout .

To see which case happens, the CL ScopedLogSeverity to output
verbose logs and lets FuseBridgeLoop skip a request from /dev/fuse if unique=0.

Bug: 62429763
Test: libappfuse_tests
Change-Id: I8c4d532564b690d55573b92260170b0cd68150ab
2017-06-20 00:25:28 +00:00
Daichi Hirono
e5a1556d40 Add more logs for writing failures.
Bug: 62429763
Test: libappfuse_test
Change-Id: Ie0eabd09ae9ad3f8ba8c4f38f871dad16b5c58ff
2017-06-12 10:37:08 +09:00
Daichi Hirono
4847c7f708 Merge "Change the CHECK failure into function failure." into oc-dev
am: bf6e949727

Change-Id: Ic374fc076cc9584c375618a57371669c23aff539
2017-05-17 10:39:16 +00:00
Daichi Hirono
d6b71b9153 Merge "Use SO_SNDBUFFORCE instead of SO_SNDBUF" into oc-dev
am: 165dad791d

Change-Id: I2da13af0ff1e4a50259847fd57695c3d616e59cc
2017-05-17 10:38:32 +00:00
TreeHugger Robot
bf6e949727 Merge "Change the CHECK failure into function failure." into oc-dev 2017-05-17 03:22:27 +00:00
Daichi Hirono
3df060d6d0 Change the CHECK failure into function failure.
Previously we have CHECK in WriteInternal function to observe short
writing. However it turns out short write can happen according to the
bug report.

To prevent app from crashing due to CHECK failure, the CL removes the
CHECK and let WriteInternal return a failure value.

Bug: 37561460
Test: libappfuse_tests, manually re-wrote the return value of write()
      and checked logcat.
Change-Id: I6a1e233c3ddb8eb68f59e7c606ad0459b5ca2c6e
2017-05-17 10:30:02 +09:00
Daichi Hirono
287776ddf4 Use SO_SNDBUFFORCE instead of SO_SNDBUF
When /proc/sys/net/core/wmem_max is smaller than kMaxMessageSize, we
need to override the limitation.

Bug: 37561460
Test: libappfuse_tests

Change-Id: Ibaac8db61290d661459fdc46f0ae8416f7db1d9e
2017-04-28 17:03:06 +09:00
Dan Shi
8a514bc30b Merge "Add test config to libappfuse_test" am: dccf5a1dc8 am: 31d4b50cc4
am: b44505e956

Change-Id: Iefe160951c3f638faee7e8e1f1ff63ed9133325c
2017-04-05 00:19:53 +00:00
Dan Shi
688b6e5e6e Add test config to libappfuse_test
This change allows TradeFederation to run the test directly.
Refer to b/35882476 for design and discussion of this change.

Bug: 35882476
Test: local test
tradefed.sh run template/local --template:map test=libappfuse_test

Change-Id: Ic3a23d9d609036658fd37fc72571e2fc7db0e88d
2017-03-31 17:11:50 -07:00
Daichi Hirono
6f6210dd5c Retry write operation when getting ENOBUFS.
Previously libappfuse set SO_SNDBUF to the maximum message size. However
it does not prevent ENOBUF and it made AppFusePerfTest#testReadWriteFile
flaky.

The CL let FuseBuffer retry write operation when getting ENOBUFS.

Bug: 34903085
Test: libappfuse
Change-Id: I1602474d852e1599f6e69103bcf6f18277a5644b
2017-03-31 01:50:50 +00:00
Daichi Hirono
8e16ceecb7 Change FuseAppLoop so that it can process messages asynchronously.
Previously FuseAppLoopCallback needs to return values in a synchrnous
manner. The CL changes it to asynchronous mannger so that apps can
process FUSE message asynchrnously.

Bug: 35229514
Test: FuseAppLoopTest
Change-Id: I8edcfdb003a25cfd5e9c490ec871140220b21e35
(cherry picked from commit f5d15f9fc4)
2017-03-29 00:13:58 +00:00
Daichi Hirono
96c6aa4f20 Enable FuseBridgeLoop to accept new mount point after starting
The CL turns StartFuseBridgeLoop function into FuseBridgeLoop class, and
adds a method adding new appfuse mount to the loop.

After doing this, one FuseBridgeLoop can process FUSE commands from
multiple AppFuse mounts.

Bug: 34903085
Test: libappfuse_test
Change-Id: I54f11f54bc26c551281b9c32e9bb91f8f043774c
2017-03-23 16:22:03 +09:00
Daichi Hirono
0bb22bd50e Add FuseMessage::WriteWithBody function
The funciton is going to be used to write FUSE header with external
body.

Bug: 35229514
Test: libappfuse_tests
Change-Id: I303022b555deca960b8e08f26140a5ef10133efe
2017-03-23 01:04:15 +00:00
Daichi Hirono
2a14b71f14 Add new EpollController class.
The class is a thin wrapper for C epoll functions.

Bug: 34903085
Test: Build EpollController.cc and libappfuse_test after applying
      future changes locally.

Change-Id: Iedce7f35e4397f80cde1054d53261ad94f9e58a8
2017-03-15 15:58:06 +09:00
Daichi Hirono
57b780fbc3 Add ReadOrAgain and WriteOrAgain methods to FuseMessage.
These methods return kAgain if operation cannot be done without blocking
the current thread.

The CL also introduecs new helper function SetupMessageSockets so that
FuseMessages are always transfered via sockets that save message
boundaries.

Bug: 34903085
Test: libappfuse_test
Change-Id: I34544372cc1b0c7bc9622e581ae16c018a123fa9
2017-03-14 09:09:29 +09:00
George Burgess IV
d6f2e69624 Add permission bits to open() with O_CREAT.
It's an error to pass open O_CREAT without giving it mode bits:
https://linux.die.net/man/3/open

Bug: 32073964
Test: Compiles with clang FORTIFY.
Change-Id: I6b2a3694f85565afdeb782585c6af36e8c4d1557
2017-02-08 23:44:29 -08:00
Daichi Hirono
cb9153bf43 Support SOCK_STREAM for bridge between system and app
Previously AppFuse use SOCK_SEQPACKET for sockets communicating system
and app. However SOCK_SEQPACKET requires the buffer of message size in
the kernel and sometimes failed to write with ENOBUF.

The CL updates libappfuse so that it can use SOCK_STREAM instead of
SOCK_SEQPACKET.

Bug: 33279206
Test: libappfuse_test
Change-Id: I622ada9ac1d71d0c57b6cfff0904c7829cea7995
2016-12-12 14:53:03 +09:00
Daichi Hirono
a6373ec1d4 Fix checks for reading and writing FuseMessage.
Previously FuseMessage were checking result of read/write operation
after checking header.len value is valid. This was wrong because
header.len does not contain correct value when read function does not
read any bytes and returns zero.

Bug: 33278098
Test: libappfuse_test
Change-Id: Icf998ca6c3eeee20cbc4aa2f65195a87e59ffc27
2016-12-12 14:34:29 +09:00
Daichi Hirono
691739b334 Remove FuseBridgeLoop class.
Bug: 32779923
Test: libappfuse_test
Change-Id: I29a76701d141ae061ec1fe32993d27460a0c694a
2016-11-17 09:49:51 +09:00
Daichi Hirono
30e6808895 Stops the loop when all files are closed.
The CL changes FuseBridgeLoop so that it exits when all files opened on
the AppFuse mount point are closed. Note that the client code will
unmount the FUSE mount point after the loop exits.

Bug: 32260320
Test: libappfuse_test
Change-Id: I4965fbb48de8a280c6306e70757a07376b1956a7
2016-11-17 09:10:53 +09:00
Treehugger Robot
c884f80755 Merge "Use FUSE_COMPAT_22_INIT_OUT_SIZE always if available." 2016-11-16 03:57:46 +00:00
Daichi Hirono
471ad6a59d Use FUSE_COMPAT_22_INIT_OUT_SIZE always if available.
We return the minor version number 15 to FUSE_INIT since we don't handle
BATCH_FORGET. Thus the kernel does not accept the latest size of
fuse_init_out. Instead we need to use FUSE_COMPAT_22_INIT_OUT_SIZE.

Bug: 32779923
Test: libappfuse_test

Change-Id: I5c979d0e45344ca8adfe3ad3f4a9561442abcb3a
2016-11-16 01:56:00 +00:00
Daichi Hirono
0d97be4d7d Add static assert to check if FuseBuffer is standard layout union.
Bug: 32260320
Test: libappfuse_test
Change-Id: I6430c11fdeb2405996410c97044b4260c25209b8
2016-11-15 10:18:37 +09:00
Daichi Hirono
a0aecda12b Add FuseAppLoop to libappfuse.
The class is used at the app side (StorageManager) to parse FUSE
commands.

Bug: 32260320
Test: libappfuse_test
Change-Id: I1ae2904d3290a041f1efbf8fc10ba032eda5449c
2016-11-15 09:47:31 +09:00
Daichi Hirono
c613476297 Add FuseBridgeLoop to libappfuse.
The CL adds FuseBridgeLoop class to libappfuse, which is used in the
system service to proxy fuse commands to applications.

Bug: 29970149
Test: libappfuse_test
Change-Id: I0708f608b3868721ab16ba4028fd2c17a6735af7
2016-10-27 15:04:15 +09:00
Daichi Hirono
7f8e819ded Add utility functions for FUSE.
The CL adds utility functions to framework to parse FUSE messages
from the kernel. The library will be used from framework JNI and service
JNI.

Bug: 32260320
Test: libappfuse_test
Change-Id: Ib89b26d34789e6c26a3288beceb3ea145c1ae780
2016-10-27 12:40:24 +09:00