Merge "fuse_sideload: Change the minimal block size to 4096." am: 4e8e56eaea am: 53b98de866

am: 6bea344d7e

Change-Id: If7e905a7630ee8cf142752695272afb8e76c28cb

Android.mk

@@ -25,7 +25,9 @@ LOCAL_SRC_FILES := fuse_sideload.cpp
 LOCAL_CFLAGS := -Wall -Werror
 LOCAL_CFLAGS += -D_XOPEN_SOURCE -D_GNU_SOURCE
 LOCAL_MODULE := libfusesideload
-LOCAL_STATIC_LIBRARIES := libcrypto
+LOCAL_STATIC_LIBRARIES := \
+    libcrypto \
+    libbase
 include $(BUILD_STATIC_LIBRARY)
 
 # libmounts (static library)

fuse_sideload.cpp

@@ -61,6 +61,9 @@
 #include <sys/uio.h>
 #include <unistd.h>
 
+#include <string>
+
+#include <android-base/stringprintf.h>
 #include <openssl/sha.h>
 
 #include "fuse_sideload.h"
@@ -364,33 +367,31 @@ static int handle_read(void* data, struct fuse_data* fd, const struct fuse_in_he
     return NO_STATUS;
 }
 
-int run_fuse_sideload(struct provider_vtab* vtab, void* cookie,
-                      uint64_t file_size, uint32_t block_size)
-{
-    int result;
-
-    // If something's already mounted on our mountpoint, try to remove
-    // it.  (Mostly in case of a previous abnormal exit.)
+int run_fuse_sideload(struct provider_vtab* vtab, void* cookie, uint64_t file_size,
+                      uint32_t block_size) {
+  // If something's already mounted on our mountpoint, try to remove it. (Mostly in case of a
+  // previous abnormal exit.)
   umount2(FUSE_SIDELOAD_HOST_MOUNTPOINT, MNT_FORCE);
 
-    if (block_size < 1024) {
+  // fs/fuse/inode.c in kernel code uses the greater of 4096 and the passed-in max_read.
+  if (block_size < 4096) {
     fprintf(stderr, "block size (%u) is too small\n", block_size);
     return -1;
   }
-    if (block_size > (1<<22)) {   // 4 MiB
+  if (block_size > (1 << 22)) {  // 4 MiB
     fprintf(stderr, "block size (%u) is too large\n", block_size);
     return -1;
   }
 
-    struct fuse_data fd;
-    memset(&fd, 0, sizeof(fd));
+  struct fuse_data fd = {};
   fd.vtab = vtab;
   fd.cookie = cookie;
   fd.file_size = file_size;
   fd.block_size = block_size;
-    fd.file_blocks = (file_size == 0) ? 0 : (((file_size-1) / block_size) + 1);
+  fd.file_blocks = (file_size == 0) ? 0 : (((file_size - 1) / block_size) + 1);
 
-    if (fd.file_blocks > (1<<18)) {
+  int result;
+  if (fd.file_blocks > (1 << 18)) {
     fprintf(stderr, "file has too many blocks (%u)\n", fd.file_blocks);
     result = -1;
     goto done;
@@ -428,19 +429,20 @@ int run_fuse_sideload(struct provider_vtab* vtab, void* cookie,
     goto done;
   }
 
-    char opts[256];
-    snprintf(opts, sizeof(opts),
-             ("fd=%d,user_id=%d,group_id=%d,max_read=%u,"
-              "allow_other,rootmode=040000"),
-             fd.ffd, fd.uid, fd.gid, block_size);
+  {
+    std::string opts = android::base::StringPrintf(
+        "fd=%d,user_id=%d,group_id=%d,max_read=%u,allow_other,rootmode=040000", fd.ffd, fd.uid,
+        fd.gid, block_size);
 
-    result = mount("/dev/fuse", FUSE_SIDELOAD_HOST_MOUNTPOINT,
-                   "fuse", MS_NOSUID | MS_NODEV | MS_RDONLY | MS_NOEXEC, opts);
+    result = mount("/dev/fuse", FUSE_SIDELOAD_HOST_MOUNTPOINT, "fuse",
+                   MS_NOSUID | MS_NODEV | MS_RDONLY | MS_NOEXEC, opts.c_str());
     if (result < 0) {
       perror("mount");
       goto done;
     }
-    uint8_t request_buffer[sizeof(struct fuse_in_header) + PATH_MAX*8];
+  }
+
+  uint8_t request_buffer[sizeof(struct fuse_in_header) + PATH_MAX * 8];
   for (;;) {
     ssize_t len = TEMP_FAILURE_RETRY(read(fd.ffd, request_buffer, sizeof(request_buffer)));
     if (len == -1) {
@@ -452,12 +454,12 @@ int run_fuse_sideload(struct provider_vtab* vtab, void* cookie,
       continue;
     }
 
-        if ((size_t)len < sizeof(struct fuse_in_header)) {
-            fprintf(stderr, "request too short: len=%zu\n", (size_t)len);
+    if (static_cast<size_t>(len) < sizeof(struct fuse_in_header)) {
+      fprintf(stderr, "request too short: len=%zd\n", len);
       continue;
     }
 
-        struct fuse_in_header* hdr = (struct fuse_in_header*) request_buffer;
+    struct fuse_in_header* hdr = reinterpret_cast<struct fuse_in_header*>(request_buffer);
     void* data = request_buffer + sizeof(struct fuse_in_header);
 
     result = -ENOSYS;
@@ -510,7 +512,7 @@ int run_fuse_sideload(struct provider_vtab* vtab, void* cookie,
     }
   }
 
-  done:
+done:
   fd.vtab->close(fd.cookie);
 
   result = umount2(FUSE_SIDELOAD_HOST_MOUNTPOINT, MNT_DETACH);

tests/Android.mk

@@ -126,6 +126,7 @@ LOCAL_STATIC_LIBRARIES := \
     libimgpatch \
     libbsdiff \
     libbspatch \
+    libfusesideload \
     libotafault \
     librecovery \
     libupdater \

tests/component/sideload_test.cpp

@@ -13,9 +13,24 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
 #include <unistd.h>
+
 #include <gtest/gtest.h>
 
-TEST(SideloadTest, fusedevice) {
-  ASSERT_NE(-1, access("/dev/fuse", R_OK | W_OK));
+#include "fuse_sideload.h"
+
+TEST(SideloadTest, fuse_device) {
+  ASSERT_EQ(0, access("/dev/fuse", R_OK | W_OK));
+}
+
+TEST(SideloadTest, run_fuse_sideload_wrong_parameters) {
+  provider_vtab vtab;
+  vtab.close = [](void*) {};
+
+  ASSERT_EQ(-1, run_fuse_sideload(&vtab, nullptr, 4096, 4095));
+  ASSERT_EQ(-1, run_fuse_sideload(&vtab, nullptr, 4096, (1 << 22) + 1));
+
+  // Too many blocks.
+  ASSERT_EQ(-1, run_fuse_sideload(&vtab, nullptr, ((1 << 18) + 1) * 4096, 4096));
 }