libfec: Fix unaligned multiblock reads

fec_process miscalculates the number of blocks it needs to read when
we perform a small read that spans multiple blocks, which results in
a partial read. The function also returns early when it notices the
partial read without cleaning up the worker threads, which can lead to
the caller releasing the buffer that the threads are still using. Fix
the calculation and add a test case to fec_unittest.

Bug: 279705058
Test: fec_unittest
Change-Id: I0d36950d3324572043b345997a21bb3e985968e5

2 files changed, 19 insertions, 14 deletions

diff --git a/libfec/fec_process.cpp b/libfec/fec_process.cpp
index f11b8b2c..46f48bc7 100644
--- a/libfec/fec_process.cpp
+++ b/libfec/fec_process.cpp
@@ -60,30 +60,27 @@ ssize_t process(fec_handle *f, uint8_t *buf, size_t count, uint64_t offset,
     }
 
     uint64_t start = (offset / FEC_BLOCKSIZE) * FEC_BLOCKSIZE;
-    size_t blocks = fec_div_round_up(count, FEC_BLOCKSIZE);
+    size_t blocks = fec_div_round_up(offset + count - start, FEC_BLOCKSIZE);
 
-    size_t count_per_thread = fec_div_round_up(blocks, threads) * FEC_BLOCKSIZE;
-    size_t max_threads = fec_div_round_up(count, count_per_thread);
-
-    if ((size_t)threads > max_threads) {
-        threads = (int)max_threads;
+    /* start at most one thread per block we're accessing */
+    if ((size_t)threads > blocks) {
+        threads = (int)blocks;
     }
 
+    size_t count_per_thread = fec_div_round_up(blocks, threads) * FEC_BLOCKSIZE;
     size_t left = count;
     uint64_t pos = offset;
     uint64_t end = start + count_per_thread;
 
-    debug("%d threads, %zu bytes per thread (total %zu)", threads,
-        count_per_thread, count);
+    debug("max %d threads, %zu bytes per thread (total %zu spanning %zu blocks)", threads,
+          count_per_thread, count, blocks);
 
     std::vector<pthread_t> handles;
     process_info info[threads];
     ssize_t rc = 0;
 
     /* start threads to process queue */
-    for (int i = 0; i < threads; ++i) {
-        check(left > 0);
-
+    for (int i = 0; i < threads && left > 0; ++i) {
         info[i].id = i;
         info[i].f = f;
         info[i].buf = &buf[pos - offset];
@@ -111,8 +108,6 @@ ssize_t process(fec_handle *f, uint8_t *buf, size_t count, uint64_t offset,
         left -= info[i].count;
     }
 
-    check(left == 0);
-
     ssize_t nread = 0;
 
     /* wait for all threads to complete */
@@ -130,7 +125,7 @@ ssize_t process(fec_handle *f, uint8_t *buf, size_t count, uint64_t offset,
         }
     }
 
-    if (rc == -1) {
+    if (left > 0 || rc == -1) {
         errno = EIO;
         return -1;
     }
diff --git a/libfec/test/fec_unittest.cpp b/libfec/test/fec_unittest.cpp
index 421eb501..1ced2d98 100644
--- a/libfec/test/fec_unittest.cpp
+++ b/libfec/test/fec_unittest.cpp
@@ -215,6 +215,16 @@ TEST_F(FecUnitTest, VerityImage_FecRead) {
 
     ASSERT_EQ(1024, fec_pread(handle, read_data.data(), 1024, corrupt_offset));
     ASSERT_EQ(std::vector<uint8_t>(1024, 255), read_data);
+
+    // Unaligned read that spans two blocks
+    ASSERT_EQ(678, fec_pread(handle, read_data.data(), 678, corrupt_offset - 123));
+    ASSERT_EQ(std::vector<uint8_t>(123, 254),
+              std::vector<uint8_t>(read_data.begin(), read_data.begin() + 123));
+    ASSERT_EQ(std::vector<uint8_t>(555, 255),
+              std::vector<uint8_t>(read_data.begin() + 123, read_data.begin() + 678));
+
+    std::vector<uint8_t> large_data(53388, 0);
+    ASSERT_EQ(53388, fec_pread(handle, large_data.data(), 53388, 385132));
 }
 
 TEST_F(FecUnitTest, LoadAvbImage_HashtreeFooter) {