From e18c03165b3ea86f563e104f44696d5c53f5b6a5 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 23 Apr 2018 15:15:40 -0700 Subject: [PATCH 1/2] libsparse: Use 'size_t' for the 'len' parameter in callbacks. This CL updates the callback function signature in sparse_file_callback() and sparse_file_foreach_chunk(). Before: int sparse_file_callback( struct sparse_file *s, bool sparse, bool crc, int (*write)(void *priv, const void *data, int len), void *priv); int sparse_file_foreach_chunk( struct sparse_file *s, bool sparse, bool crc, int (*write)( void *priv, const void *data, int len, unsigned int block, unsigned int nr_blocks), void *priv); After: int sparse_file_callback( struct sparse_file *s, bool sparse, bool crc, int (*write)(void *priv, const void *data, size_t len), void *priv); int sparse_file_foreach_chunk( struct sparse_file *s, bool sparse, bool crc, int (*write)( void *priv, const void *data, size_t len, unsigned int block, unsigned int nr_blocks), void *priv); The length (i.e. 'len') comes from the size of a chunk, which could be legitimately larger than INT_MAX. Prior to this CL, callers (e.g. write_sparse_data_chunk()) were already passing unsigned int to the callbacks. When a chunk size exceeds INT_MAX, the callback would see a negative value, which could lead to undesired behavior. For example, out_counter_write(), as one of the internal callbacks in libsparse, gives a wrong sum of chunk sizes, which in turn fails the fastboot flashing when given a huge sparse image. It also defines SPARSE_CALLBACK_USES_SIZE_T that allows clients to keep their codes compatibile with both versions. Bug: 78432315 Test: `m dist` (with matching changes to all the clients) Test: Build fastboot and successfully flash a previously failing (huge) sparse image. Change-Id: Iac4bcf7b57039d08af3c57f4be00d75f6b693d93 --- libsparse/include/sparse/sparse.h | 10 ++++++++-- libsparse/output_file.c | 5 +++-- libsparse/output_file.h | 3 ++- libsparse/sparse.c | 12 ++++++------ 4 files changed, 19 insertions(+), 11 deletions(-) diff --git a/libsparse/include/sparse/sparse.h b/libsparse/include/sparse/sparse.h index 356f65fd0..1b91ead5c 100644 --- a/libsparse/include/sparse/sparse.h +++ b/libsparse/include/sparse/sparse.h @@ -18,6 +18,7 @@ #define _LIBSPARSE_SPARSE_H_ #include +#include #include #ifdef __cplusplus @@ -26,6 +27,11 @@ extern "C" { struct sparse_file; +// The callbacks in sparse_file_callback() and sparse_file_foreach_chunk() take +// size_t as the length type (was `int` in past). This allows clients to keep +// their codes compatibile with both versions as needed. +#define SPARSE_CALLBACK_USES_SIZE_T + /** * sparse_file_new - create a new sparse file cookie * @@ -201,7 +207,7 @@ unsigned int sparse_file_block_size(struct sparse_file *s); * Returns 0 on success, negative errno on error. */ int sparse_file_callback(struct sparse_file *s, bool sparse, bool crc, - int (*write)(void *priv, const void *data, int len), void *priv); + int (*write)(void *priv, const void *data, size_t len), void *priv); /** * sparse_file_foreach_chunk - call a callback for data blocks in sparse file @@ -218,7 +224,7 @@ int sparse_file_callback(struct sparse_file *s, bool sparse, bool crc, * Returns 0 on success, negative errno on error. */ int sparse_file_foreach_chunk(struct sparse_file *s, bool sparse, bool crc, - int (*write)(void *priv, const void *data, int len, unsigned int block, + int (*write)(void *priv, const void *data, size_t len, unsigned int block, unsigned int nr_blocks), void *priv); /** diff --git a/libsparse/output_file.c b/libsparse/output_file.c index 51e60ef79..002ad2746 100644 --- a/libsparse/output_file.c +++ b/libsparse/output_file.c @@ -109,7 +109,7 @@ struct output_file_normal { struct output_file_callback { struct output_file out; void *priv; - int (*write)(void *priv, const void *buf, int len); + int (*write)(void *priv, const void *buf, size_t len); }; #define to_output_file_callback(_o) \ @@ -634,7 +634,8 @@ static struct output_file *output_file_new_normal(void) return &outn->out; } -struct output_file *output_file_open_callback(int (*write)(void *, const void *, int), +struct output_file *output_file_open_callback( + int (*write)(void *, const void *, size_t), void *priv, unsigned int block_size, int64_t len, int gz __unused, int sparse, int chunks, int crc) { diff --git a/libsparse/output_file.h b/libsparse/output_file.h index b67e94ecb..690f61057 100644 --- a/libsparse/output_file.h +++ b/libsparse/output_file.h @@ -27,7 +27,8 @@ struct output_file; struct output_file *output_file_open_fd(int fd, unsigned int block_size, int64_t len, int gz, int sparse, int chunks, int crc); -struct output_file *output_file_open_callback(int (*write)(void *, const void *, int), +struct output_file *output_file_open_callback( + int (*write)(void *, const void *, size_t), void *priv, unsigned int block_size, int64_t len, int gz, int sparse, int chunks, int crc); int write_data_chunk(struct output_file *out, unsigned int len, void *data); diff --git a/libsparse/sparse.c b/libsparse/sparse.c index b17586066..466435f7b 100644 --- a/libsparse/sparse.c +++ b/libsparse/sparse.c @@ -179,7 +179,7 @@ int sparse_file_write(struct sparse_file *s, int fd, bool gz, bool sparse, } int sparse_file_callback(struct sparse_file *s, bool sparse, bool crc, - int (*write)(void *priv, const void *data, int len), void *priv) + int (*write)(void *priv, const void *data, size_t len), void *priv) { int ret; int chunks; @@ -203,11 +203,11 @@ struct chunk_data { void *priv; unsigned int block; unsigned int nr_blocks; - int (*write)(void *priv, const void *data, int len, unsigned int block, - unsigned int nr_blocks); + int (*write)(void *priv, const void *data, size_t len, + unsigned int block, unsigned int nr_blocks); }; -static int foreach_chunk_write(void *priv, const void *data, int len) +static int foreach_chunk_write(void *priv, const void *data, size_t len) { struct chunk_data *chk = priv; @@ -215,7 +215,7 @@ static int foreach_chunk_write(void *priv, const void *data, int len) } int sparse_file_foreach_chunk(struct sparse_file *s, bool sparse, bool crc, - int (*write)(void *priv, const void *data, int len, unsigned int block, + int (*write)(void *priv, const void *data, size_t len, unsigned int block, unsigned int nr_blocks), void *priv) { @@ -250,7 +250,7 @@ int sparse_file_foreach_chunk(struct sparse_file *s, bool sparse, bool crc, return ret; } -static int out_counter_write(void *priv, const void *data __unused, int len) +static int out_counter_write(void *priv, const void *data __unused, size_t len) { int64_t *count = priv; *count += len; From 7d27ffdd4584990ad3bc2415e188718110b44e31 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 23 Apr 2018 17:24:17 -0700 Subject: [PATCH 2/2] fastboot: Track the libsparse API change. Bug: 78432315 Test: Successfully flash a previously failing (huge) sparse image. Test: `fastboot update` existing marlin-img.zip. Change-Id: I09c9a06109769882d26be56d4a0d2a2b7b62cb5f --- fastboot/protocol.cpp | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/fastboot/protocol.cpp b/fastboot/protocol.cpp index 7a333ee06..fda6f5dcb 100644 --- a/fastboot/protocol.cpp +++ b/fastboot/protocol.cpp @@ -278,19 +278,14 @@ int64_t fb_upload_data(Transport* transport, const char* outfile) { return _command_end(transport); } -#define TRANSPORT_BUF_SIZE 1024 +static constexpr size_t TRANSPORT_BUF_SIZE = 1024; static char transport_buf[TRANSPORT_BUF_SIZE]; -static int transport_buf_len; - -static int fb_download_data_sparse_write(void *priv, const void *data, int len) -{ - int r; - Transport* transport = reinterpret_cast(priv); - int to_write; - const char* ptr = reinterpret_cast(data); +static size_t transport_buf_len; +static int fb_download_data_sparse_write(void* priv, const void* data, size_t len) { + const char* ptr = static_cast(data); if (transport_buf_len) { - to_write = std::min(TRANSPORT_BUF_SIZE - transport_buf_len, len); + size_t to_write = std::min(TRANSPORT_BUF_SIZE - transport_buf_len, len); memcpy(transport_buf + transport_buf_len, ptr, to_write); transport_buf_len += to_write; @@ -298,9 +293,10 @@ static int fb_download_data_sparse_write(void *priv, const void *data, int len) len -= to_write; } + Transport* transport = static_cast(priv); if (transport_buf_len == TRANSPORT_BUF_SIZE) { - r = _command_write_data(transport, transport_buf, TRANSPORT_BUF_SIZE); - if (r != TRANSPORT_BUF_SIZE) { + int64_t r = _command_write_data(transport, transport_buf, TRANSPORT_BUF_SIZE); + if (r != static_cast(TRANSPORT_BUF_SIZE)) { return -1; } transport_buf_len = 0; @@ -311,9 +307,9 @@ static int fb_download_data_sparse_write(void *priv, const void *data, int len) g_error = "internal error: transport_buf not empty"; return -1; } - to_write = round_down(len, TRANSPORT_BUF_SIZE); - r = _command_write_data(transport, ptr, to_write); - if (r != to_write) { + size_t to_write = round_down(len, TRANSPORT_BUF_SIZE); + int64_t r = _command_write_data(transport, ptr, to_write); + if (r != static_cast(to_write)) { return -1; } ptr += to_write;