[PATCH RFC 0/9] Proof of concept of chunked checksums

[Dominique Martinet]

This is the poc I had promised in my mail last week:
https://groups.google.com/g/swupdate/c/sNnb1SvHPxw/m/8qjMm7JYAAAJ

There's quite a bit of cleanup first that would probably be good to take
regardless -- Stefano, do you want me to resend the fixes / cleanup
separately first?

The core of the implementation itself is in patches 7 & 8;
7 (adding a step to cpio_utils) is as I had expected fairly
straightforward, we'll probably want to make the chunk size configurable
and perhaps change the chunk implementation if mmap causes issues but it
wasn't too bad.
8 (config) is ugly because we're parsing it all upfront, I had suggested
keeping a reference to the config in img and freeing it in free_img
instead and that's still possible but ultimately keeping a handle to the
config keeps the whole data as string in memory so it's better to
re-alloc something more efficient and move it there; keeping a reference
to the config handle also wouldn't work well with the lua external
parsing so this is probably better this way.
(I haven't implemented the external parser C-lua glue here, but will do
before sending the final patch if we go that way -- I've never used it
though so might need help testing)


Dominique Martinet (9):
parser: set_find_path: properly return to-free string array
cpio_utils: move out hash comparison
copyfile cleanup: use copyimage when we can
copyfile refactor: replace the 13 arguments by a struct
cpio_utils: cleanup state chaining code
cpio_utils: fill_buffer: fix return value on EOF after short read
cpio_utils: implement chunked hash verification
config parsing: add handling for chunked_hashes
cpio_utils: skip full sha256 if chunked hash is set

Kconfig | 4 +
core/cpio_utils.c | 383 ++++++++++++++++++----------
core/installer.c | 20 +-
core/parsing_library.c | 55 +++-
core/stream_interface.c | 24 +-
corelib/lua_interface.c | 38 +--
corelib/parsing_library_libconfig.c | 2 +-
corelib/parsing_library_libjson.c | 2 +-
handlers/copy_handler.c | 20 +-
handlers/delta_handler.c | 42 ++-
handlers/rdiff_handler.c | 13 +-
handlers/readback_handler.c | 20 +-
include/parselib.h | 5 +-
include/swupdate_image.h | 1 +
include/util.h | 42 ++-
parser/parser.c | 7 +-
16 files changed, 422 insertions(+), 256 deletions(-)

--
2.39.2

[Dominique Martinet]

- reword comment about 64 bytes for md_value
- only convert to ascii if we're going to use it for logging

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---
core/cpio_utils.c | 65 +++++++++++++++++++++++++----------------------
1 file changed, 35 insertions(+), 30 deletions(-)

diff --git a/core/cpio_utils.c b/core/cpio_utils.c
index 00b15f030c74..a05a32cd59b5 100644
--- a/core/cpio_utils.c
+++ b/core/cpio_utils.c
@@ -434,6 +434,38 @@ static int zstd_step(void* state, void* buffer, size_t size)

#endif

+static int hash_compare(struct swupdate_digest *dgst, unsigned char *hash)
+{
+ /*
+ * SHA256_HASH_LENGTH should be enough but openssl might write
+ * up to EVP_MAX_MD_SIZE = 64 bytes (sha512 size)
+ */
+ unsigned char md_value[64];
+ unsigned int md_len = 0;
+
+ if (swupdate_HASH_final(dgst, md_value, &md_len) < 0) {
+ return -EFAULT;
+ }
+ /*
+ * Now check if the computed hash is equal
+ * to the value retrieved from sw-descritpion
+ */
+ if (md_len != SHA256_HASH_LENGTH || swupdate_HASH_compare(hash, md_value)) {
+#ifndef CONFIG_ENCRYPTED_IMAGES_HARDEN_LOGGING
+ char hashstring[2 * SHA256_HASH_LENGTH + 1];
+ char newhashstring[2 * SHA256_HASH_LENGTH + 1];
+
+ hash_to_ascii(hash, hashstring);
+ hash_to_ascii(md_value, newhashstring);
+
+ ERROR("HASH mismatch : %s <--> %s",
+ hashstring, newhashstring);
+#endif
+ return -EFAULT;
+ }
+ return 0;
+}
+
static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nbytes, unsigned long *offs, unsigned long long seek,
int skip_file, int __attribute__ ((__unused__)) compressed,
uint32_t *checksum, unsigned char *hash, bool encrypted, const char *imgivt, writeimage callback)
@@ -441,11 +473,6 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
unsigned int percent, prevpercent = 0;
int ret = 0;
int len;
- unsigned char md_value[64]; /*
- * Maximum hash is 64 bytes for SHA512
- * and we use sha256 in swupdate
- */
- unsigned int md_len = 0;
unsigned char *aes_key = NULL;
unsigned char *ivt = NULL;
unsigned char ivtbuf[AES_BLK_SIZE];
@@ -648,31 +675,9 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
}
}

- if (IsValidHash(hash)) {
- if (swupdate_HASH_final(input_state.dgst, md_value, &md_len) < 0) {
- ret = -EFAULT;
- goto copyfile_exit;
- }
-
-
- /*
- * Now check if the computed hash is equal
- * to the value retrieved from sw-descritpion
- */
- if (md_len != SHA256_HASH_LENGTH || swupdate_HASH_compare(hash, md_value)) {
- char hashstring[2 * SHA256_HASH_LENGTH + 1];
- char newhashstring[2 * SHA256_HASH_LENGTH + 1];
-
- hash_to_ascii(hash, hashstring);
- hash_to_ascii(md_value, newhashstring);
-
-#ifndef CONFIG_ENCRYPTED_IMAGES_HARDEN_LOGGING
- ERROR("HASH mismatch : %s <--> %s",
- hashstring, newhashstring);
-#endif
- ret = -EFAULT;
- goto copyfile_exit;
- }
+ if (IsValidHash(hash) && hash_compare(input_state.dgst, hash) < 0) {
+ ret = -EFAULT;
+ goto copyfile_exit;
}

if (!inbuf) {
--
2.39.2

[Dominique Martinet]

properly set 'tmp' through an extra indirection instead of leaking
the string array

Reported-by: gcc -fanalyzer

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---

core/parsing_library.c | 5 ++---

corelib/parsing_library_libconfig.c | 2 +-
corelib/parsing_library_libjson.c | 2 +-

include/parselib.h | 2 +-
parser/parser.c | 2 +-
5 files changed, 6 insertions(+), 7 deletions(-)

diff --git a/core/parsing_library.c b/core/parsing_library.c
index a97f8114becc..038c6851adf9 100644
--- a/core/parsing_library.c
+++ b/core/parsing_library.c
@@ -210,7 +210,7 @@ void get_hash_value(parsertype p, void *elem, unsigned char *hash)
ascii_to_hash(hash, hash_ascii);
}

-bool set_find_path(const char **nodes, const char *newpath, char **tmp)
+bool set_find_path(const char **nodes, const char *newpath, char ***tmp)
{
char **paths;
unsigned int count;
@@ -218,7 +218,6 @@ bool set_find_path(const char **nodes, const char *newpath, char **tmp)
char *token, *ref;
bool first = true;
int allocstr = 0;
- (void)tmp;

/*
* Include of files is not supported,
@@ -301,7 +300,7 @@ bool set_find_path(const char **nodes, const char *newpath, char **tmp)
}

free(ref);
- tmp = paths;
+ *tmp = paths;

return true;
}
diff --git a/corelib/parsing_library_libconfig.c b/corelib/parsing_library_libconfig.c
index ddb79f6fb152..4ddc338031a0 100644
--- a/corelib/parsing_library_libconfig.c
+++ b/corelib/parsing_library_libconfig.c
@@ -196,7 +196,7 @@ void *find_root_libconfig(config_t *cfg, const char **nodes, unsigned int depth)
if (elem && config_setting_is_group(elem) == CONFIG_TRUE) {
ref = get_field_string_libconfig(elem, "ref");
if (ref) {
- if (!set_find_path(nodes, ref, tmp)) {
+ if (!set_find_path(nodes, ref, &tmp)) {
free(root);
return NULL;
}
diff --git a/corelib/parsing_library_libjson.c b/corelib/parsing_library_libjson.c
index 4728d200fc8e..9456d51eca9d 100644
--- a/corelib/parsing_library_libjson.c
+++ b/corelib/parsing_library_libjson.c
@@ -237,7 +237,7 @@ void *find_root_json(json_object *root, const char **nodes, unsigned int depth)
if (type == json_type_object || type == json_type_array) {
str = get_field_string_json(node, "ref");
if (str) {
- if (!set_find_path(nodes, str, tmp))
+ if (!set_find_path(nodes, str, &tmp))
return NULL;
node = find_root_json(root, nodes, depth);
free_string_array(tmp);
diff --git a/include/parselib.h b/include/parselib.h
index 99b6519fa480..48f38d2a8c92 100644
--- a/include/parselib.h
+++ b/include/parselib.h
@@ -64,7 +64,7 @@ void get_hash_value(parsertype p, void *elem, unsigned char *hash);
void check_field_string(const char *src, char *dst, const size_t max_len);
void *find_root(parsertype p, void *root, const char **nodes);
void *get_node(parsertype p, void *root, const char **nodes);
-bool set_find_path(const char **nodes, const char *newpath, char **tmp);
+bool set_find_path(const char **nodes, const char *newpath, char ***tmp);

#define GET_FIELD_STRING(p, e, name, d) \
get_field_string_with_size(p, e, name, d, sizeof(d))
diff --git a/parser/parser.c b/parser/parser.c
index 04d7b72c9b9a..2273796e0452 100644
--- a/parser/parser.c
+++ b/parser/parser.c
@@ -144,7 +144,7 @@ static int parser_follow_link(parsertype p, void *cfg, void *elem,
for (int j = 0; j < count_string_array(nodes); j++) {
linknodes[j] = nodes[j];
}
- if (!set_find_path(linknodes, ref, tmp)) {
+ if (!set_find_path(linknodes, ref, &tmp)) {
free(linknodes);
return -1;
}
--
2.39.2

[Dominique Martinet]

There were a few handlers manually passing all the fields required for copyimage,
use copyimage instead.

lua was using img.checksum as a secondary hash, preserve this
functionality by remembering the old img.checksum instead of passing a
different checksum pointer.

There is no intended functional change.

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---

corelib/lua_interface.c | 38 ++++++++------------------------------
handlers/delta_handler.c | 13 +------------
handlers/rdiff_handler.c | 13 +------------
3 files changed, 10 insertions(+), 54 deletions(-)

diff --git a/corelib/lua_interface.c b/corelib/lua_interface.c
index 03b74aed1cf2..10af0760c5f9 100644
--- a/corelib/lua_interface.c
+++ b/corelib/lua_interface.c
@@ -388,7 +388,7 @@ static int l_copy2file(lua_State *L)
}

struct img_type img = {};
- uint32_t checksum = 0;
+ uint32_t image_checksum = img.checksum;

table2image(L, &img);
if (check_same_file(img.fdin, fdout)) {
@@ -397,18 +397,7 @@ static int l_copy2file(lua_State *L)
lua_pushstring(L, "Output file path is same as input file temporary path");
goto copyfile_exit;
}
- int ret = copyfile(img.fdin,
- &fdout,
- img.size,
- (unsigned long *)&img.offset,
- img.seek,
- 0, /* no skip */
- img.compressed,
- &checksum,
- img.sha256,
- img.is_encrypted,
- img.ivt_ascii,
- NULL);
+ int ret = copyimage(&fdout, &img, NULL /* default write callback */);
update_table(L, &img);
lua_pop(L, 1);

@@ -417,10 +406,10 @@ static int l_copy2file(lua_State *L)
lua_pushstring(L, strerror(errno));
goto copyfile_exit;
}
- if ((img.checksum != 0) && (checksum != img.checksum)) {
+ if ((image_checksum != 0) && (image_checksum != img.checksum)) {
lua_pushinteger(L, -1);
lua_pushfstring(L, "Checksums WRONG! Computed 0x%d, should be 0x%d\n",
- checksum, img.checksum);
+ img.checksum, image_checksum);
goto copyfile_exit;
}

@@ -463,24 +452,13 @@ static int l_istream_read(lua_State* L)
luaL_checktype(L, 2, LUA_TFUNCTION);

struct img_type img = {};
- uint32_t checksum = 0;
+ uint32_t image_checksum = img.checksum;

lua_pushvalue(L, 1);
table2image(L, &img);
lua_pop(L, 1);

- int ret = copyfile(img.fdin,
- L,
- img.size,
- (unsigned long *)&img.offset,
- img.seek,
- 0, /* no skip */
- img.compressed,
- &checksum,
- img.sha256,
- img.is_encrypted,
- img.ivt_ascii,
- istream_read_callback);
+ int ret = copyimage(L, &img, istream_read_callback);

lua_pop(L, 1);
update_table(L, &img);
@@ -491,10 +469,10 @@ static int l_istream_read(lua_State* L)
lua_pushstring(L, strerror(errno));
return 2;
}
- if ((img.checksum != 0) && (checksum != img.checksum)) {
+ if ((image_checksum != 0) && (image_checksum != img.checksum)) {
lua_pushinteger(L, -1);
lua_pushfstring(L, "Checksums WRONG! Computed 0x%d, should be 0x%d\n",
- checksum, img.checksum);
+ img.checksum, image_checksum);
return 2;
}
lua_pushinteger(L, 0);
diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c
index 8e7faa0ead02..5ec243ee01d9 100644
--- a/handlers/delta_handler.c
+++ b/handlers/delta_handler.c
@@ -999,18 +999,7 @@ static int install_delta(struct img_type *img,
goto cleanup;
}

- ret = copyfile(img->fdin,
- &mem_fd,
- img->size,
- (unsigned long *)&img->offset,
- img->seek,
- 0,
- img->compressed,
- &img->checksum,
- img->sha256,
- img->is_encrypted,
- img->ivt_ascii,
- NULL);
+ ret = copyimage(&mem_fd, img, NULL /* default write callback */);

if (ret != 0) {
ERROR("Error %d copying zchunk header, aborting.", ret);
diff --git a/handlers/rdiff_handler.c b/handlers/rdiff_handler.c
index e01a127bce50..3b01519ce80a 100644
--- a/handlers/rdiff_handler.c
+++ b/handlers/rdiff_handler.c
@@ -337,18 +337,7 @@ static int apply_rdiff_patch(struct img_type *img,
rs_trace_to(rdiff_log);

rdiff_state.job = rs_patch_begin(base_file_read_cb, rdiff_state.base_file);
- ret = copyfile(img->fdin,
- &rdiff_state,
- img->size,
- (unsigned long *)&img->offset,
- img->seek,
- 0, /* no skip */
- img->compressed,
- &img->checksum,
- img->sha256,
- img->is_encrypted,
- img->ivt_ascii,
- apply_rdiff_chunk_cb);
+ ret = copyimage(&rdiff_state, img, apply_rdiff_chunk_cb);
if (ret != 0) {
ERROR("Error %d running rdiff job, aborting.", ret);
goto cleanup;
--
2.39.2

[Dominique Martinet]

This allows something like this to be fed to the new chunked_hashes code
(libconfig syntax):
-----
images: (
{
filename = "somefile.tar.zst";
name = "version";
version = "1";
install-if-higher = true;
compressed = "zstd";
encrypted = true;
ivt = "3b627a40272e3ba253794680e6f870c9";
installed-directly = true;
sha256 = "e437493d207e7dabfba69997072b938bcb93175eea09d4108b96825d1fc2b079";
chunked_hashes = (
"3028e787495f7a97bfb6c10110affed016d398374d8c2c02fd7d02bc84f75e68",
"5280083a531be033af6bb31a579737fb296739c0d9fa100457ce328fdfad01d8",
);
type = "archive";
path = "/tmp/swupdate";
properties: {
create-destination = "true";
};
},
);
-----

Note the sha256 is ignored if chunked_hashes is found in copyfile(), but
we've left it so this is installable by older versions of swupdate.

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---

core/cpio_utils.c | 1 +
core/installer.c | 1 +
core/parsing_library.c | 50 ++++++++++++++++++++++++++++++++++++++++
include/parselib.h | 3 +++
include/swupdate_image.h | 1 +
parser/parser.c | 5 ++++
6 files changed, 61 insertions(+)

diff --git a/core/cpio_utils.c b/core/cpio_utils.c
index d296fe8d4525..fd52c9ef0ba6 100644
--- a/core/cpio_utils.c
+++ b/core/cpio_utils.c
@@ -847,6 +847,7 @@ int copyimage(void *out, struct img_type *img, writeimage callback)
.compressed = img->compressed,
.checksum = &img->checksum,
.hash = img->sha256,
+ .chunked_hashes = img->chunked_hashes,
.encrypted = img->is_encrypted,
.imgivt = img->ivt_ascii,
};
diff --git a/core/installer.c b/core/installer.c
index 0cb06b2ca419..2a2b0f6cf32b 100644
--- a/core/installer.c
+++ b/core/installer.c
@@ -512,6 +512,7 @@ static void cleaup_img_entry(struct img_type *img)

void free_image(struct img_type *img) {
dict_drop_db(&img->properties);
+ free(img->chunked_hashes);
free(img);
}

diff --git a/core/parsing_library.c b/core/parsing_library.c
index 038c6851adf9..b0f0e68df04e 100644
--- a/core/parsing_library.c
+++ b/core/parsing_library.c
@@ -210,6 +210,56 @@ void get_hash_value(parsertype p, void *elem, unsigned char *hash)
ascii_to_hash(hash, hash_ascii);
}

+#ifdef CONFIG_CHUNKED_HASH
+int get_chunked_hashes(parsertype p, void *elem, unsigned char **chunked_hashes)
+{
+ void *hashes_node = get_child(p, elem, "chunked_hashes");
+
+ /* does not have chunked hashes */
+ if (!hashes_node)
+ return 0;
+
+ int count = get_array_length(p, hashes_node);
+ if (!count) {
+ ERROR("chunked_hashes is not an array");
+ return -EINVAL;
+ }
+
+ unsigned char *hashes = malloc((count+1) * SHA256_HASH_LENGTH);
+ if (!hashes) {
+ ERROR("No memory: failed for %d bytes", (count+1) * SHA256_HASH_LENGTH);
+ return -ENOMEM;
+ }
+
+ int idx;
+ for (idx = 0; idx < count; idx++) {
+ void *hash_node = get_elem_from_idx(p, hashes_node, idx);
+ if (!hash_node) {
+ ERROR("Could not get %dth hash in %d long array?", idx, count);
+ free(hashes);
+ return -EINVAL;
+ }
+ const char *hash_str = get_field_string(p, hash_node, NULL);
+ if (!hash_str) {
+ ERROR("%dth hash in chunked hashes array was not a string?", idx);
+ free(hashes);
+ return -EINVAL;
+ }
+ if (ascii_to_hash(hashes + idx * SHA256_HASH_LENGTH, hash_str) < 0) {
+ ERROR("Invalid hash %s", hash_str);
+ free(hashes);
+ return -EINVAL;
+ }
+ }
+
+ /* zero final hash marking end of array */
+ memset(hashes + count * SHA256_HASH_LENGTH, 0, SHA256_HASH_LENGTH);
+
+ *chunked_hashes = hashes;
+ return 0;
+}
+#endif
+
bool set_find_path(const char **nodes, const char *newpath, char ***tmp)
{
char **paths;
diff --git a/include/parselib.h b/include/parselib.h
index 48f38d2a8c92..0a8c3842e3fb 100644
--- a/include/parselib.h
+++ b/include/parselib.h
@@ -61,6 +61,9 @@ void iterate_field(parsertype p, void *e, iterate_callback cb, void *data);
void get_field(parsertype p, void *e, const char *path, void *dest, field_type_t type);
int exist_field_string(parsertype p, void *e, const char *path);
void get_hash_value(parsertype p, void *elem, unsigned char *hash);
+#ifdef CONFIG_CHUNKED_HASH
+int get_chunked_hashes(parsertype p, void *elem, unsigned char **chunked_hashes);
+#endif

void check_field_string(const char *src, char *dst, const size_t max_len);
void *find_root(parsertype p, void *root, const char **nodes);
void *get_node(parsertype p, void *root, const char **nodes);

diff --git a/include/swupdate_image.h b/include/swupdate_image.h
index e214aafc2965..cb51da4325a3 100644
--- a/include/swupdate_image.h
+++ b/include/swupdate_image.h
@@ -75,6 +75,7 @@ struct img_type {
long long size;
unsigned int checksum;
unsigned char sha256[SHA256_HASH_LENGTH]; /* SHA-256 is 32 byte */
+ unsigned char *chunked_hashes;
LIST_ENTRY(img_type) next;
};

diff --git a/parser/parser.c b/parser/parser.c
index 2273796e0452..12420554837c 100644
--- a/parser/parser.c
+++ b/parser/parser.c
@@ -421,6 +421,11 @@ static int parse_common_attributes(parsertype p, void *elem, struct img_type *im
GET_FIELD_STRING(p, elem, "type", image->type);
GET_FIELD_STRING(p, elem, "data", image->type_data);
get_hash_value(p, elem, image->sha256);
+#ifdef CONFIG_CHUNKED_HASH
+ if (get_chunked_hashes(p, elem, &image->chunked_hashes)) {
+ return -1;
+ }
+#endif

/*
* offset can be set as number or string. As string,
--
2.39.2

[Dominique Martinet]

fill_buffer would return 0 after having filled buffer/updated checksums
if it reaches the end of file (returns 0) after another read.

In practice, this never happens because the caller knows how much to
read, and chunks are smaller than what had been previously written in
the pipe so this is a theorical problem, but because the checksums are
correctly updated until the end this would not be detected if it happens
in practice (the end of the data would just not be fed to the handler),
so this is best fixed.

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---

core/cpio_utils.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/core/cpio_utils.c b/core/cpio_utils.c
index b0a97004baf6..382b22c36434 100644
--- a/core/cpio_utils.c
+++ b/core/cpio_utils.c
@@ -73,7 +73,7 @@ static int _fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigne
return -EFAULT;
}
if (len == 0) {
- return 0;
+ return count;
}
if (checksum)
for (i = 0; i < len; i++)
--
2.39.2

[Dominique Martinet]

This is not used yet as nothing sets copyfile's chunk_hashes yet.

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---

Kconfig | 4 ++
core/cpio_utils.c | 130 ++++++++++++++++++++++++++++++++++++++++++++++
include/util.h | 2 +
3 files changed, 136 insertions(+)

diff --git a/Kconfig b/Kconfig
index 7139617c4515..f1dbe3049676 100644
--- a/Kconfig
+++ b/Kconfig
@@ -522,6 +522,10 @@ config PKCS11
key available in a file. This is implemented with wolfSSL independent
from the SSL implementation and replaces the plain key method.

+config CHUNKED_HASH
+ bool "support for chuked checksums input"
+ default y
+
comment "Compressors (zlib always on)"

config GUNZIP
diff --git a/core/cpio_utils.c b/core/cpio_utils.c
index 382b22c36434..d296fe8d4525 100644
--- a/core/cpio_utils.c
+++ b/core/cpio_utils.c
@@ -19,6 +19,7 @@
#ifdef CONFIG_ZSTD
#include <zstd.h>
#endif
+#include <sys/mman.h>

#include "generated/autoconf.h"
#include "cpiohdr.h"
@@ -466,6 +467,79 @@ static int hash_compare(struct swupdate_digest *dgst, unsigned char *hash)
return 0;
}

+#ifdef CONFIG_CHUNKED_HASH
+struct ChunkedHashState {
+ PipelineStep upstream_step;
+ void *upstream_state;
+
+ /* null-terminated array of checksums */
+ unsigned char *chunked_hashes;
+
+ /* mmaped buffer */
+ void *chunk_data;
+ size_t chunk_size;
+ size_t valid;
+
+ /* amount written */
+ size_t offset;
+};
+
+
+static int chunked_hash_step(void *_state, void *buffer, size_t size)
+{
+ struct ChunkedHashState *state = (struct ChunkedHashState *)_state;
+ int count = 0, n;
+
+ while (count < size) {
+ if (state->offset == state->valid) {
+ /* input_step does full reads */
+ if (state->valid && state->valid < state->chunk_size)
+ return count;
+
+ if (!IsValidHash(state->chunked_hashes)) {
+ ERROR("No hash left for chunk -- file too large?");

+ return -EFAULT;
+ }
+

+ n = state->upstream_step(state->upstream_state, state->chunk_data, state->chunk_size);
+ if (n <= 0) {
+ return n;
+ }
+
+ /*
+ * possible improvement: in theory we can re-init a given buffer after final(),
+ * but swupdate_HASH_* does not expose this API at this point.
+ * Realloc every time.
+ */
+ void *dgst = swupdate_HASH_init(SHA_DEFAULT);
+ if (!dgst)
+ return -EFAULT;
+
+ if (swupdate_HASH_update(dgst, state->chunk_data, n) < 0 ||
+ hash_compare(dgst, state->chunked_hashes) < 0) {
+ swupdate_HASH_cleanup(dgst);
+ return -EFAULT;
+ }
+ swupdate_HASH_cleanup(dgst);
+
+ state->chunked_hashes += SHA256_HASH_LENGTH;
+ state->valid = n;
+ state->offset = 0;
+ }
+
+ n = state->valid - state->offset;
+ if (n > size)
+ n = size;
+ memcpy(buffer + count, state->chunk_data + state->offset, n);
+ count += n;
+ size -= n;
+ state->offset += n;
+ }
+
+ return count;
+}
+#endif
+
int copyfile(struct swupdate_copy *args)
{

unsigned int percent, prevpercent = 0;

@@ -492,6 +566,10 @@ int copyfile(struct swupdate_copy *args)
.outlen = 0, .eof = false
};

+#ifdef CONFIG_CHUNKED_HASH
+ struct ChunkedHashState chunked_hash_state = { 0 };
+#endif
+
#if defined(CONFIG_GUNZIP) || defined(CONFIG_ZSTD)
struct DecompressState decompress_state = {
.upstream_step = NULL, .upstream_state = NULL,
@@ -619,6 +697,52 @@ int copyfile(struct swupdate_copy *args)
step = &input_step;
state = &input_state;

+#ifdef CONFIG_CHUNKED_HASH
+ if (args->chunked_hashes) {
+ char tmpfilename[MAX_IMAGE_FNAME];
+ int tmpfd;
+
+ /* this requires a large-ish buffer for chunk size: use a temporary file mmaped */
+ /* XXX make configurable */
+ chunked_hash_state.chunk_size = 512*1024;
+ snprintf(tmpfilename, sizeof(tmpfilename), "%s/swtmp-datachunkXXXXXX", get_tmpdir());
+ tmpfd = mkstemp(tmpfilename);
+ if (tmpfd < 0) {
+ ERROR("Could not open temporary fd, error %d", errno);

+ ret = -EFAULT;
+ goto copyfile_exit;

+ }
+ unlink(tmpfilename);
+
+ if (ftruncate(tmpfd, chunked_hash_state.chunk_size) < 0) {
+ ERROR("Could not open temporary fd, error %d", errno);
+ ret = -EFAULT;
+ close(tmpfd);
+ goto copyfile_exit;
+ }
+
+ chunked_hash_state.chunk_data =
+ mmap(NULL, chunked_hash_state.chunk_size, PROT_READ|PROT_WRITE,
+ MAP_SHARED, tmpfd, 0);
+
+ if (chunked_hash_state.chunk_data == MAP_FAILED) {
+ ERROR("Could not mmap temporary data, error %d", errno);
+ ret = -EFAULT;
+ close(tmpfd);
+ goto copyfile_exit;
+ }
+
+ /* close immediately, mmap stays valid until munmap */
+ close(tmpfd);
+ chunked_hash_state.chunked_hashes = args->chunked_hashes;
+
+ chunked_hash_state.upstream_step = step;
+ chunked_hash_state.upstream_state = state;
+ step = &chunked_hash_step;
+ state = &chunked_hash_state;
+ }
+#endif
+
if (args->encrypted) {
decrypt_state.upstream_step = step;
decrypt_state.upstream_state = state;
@@ -687,6 +811,12 @@ copyfile_exit:
if (decrypt_state.dcrypt) {
swupdate_DECRYPT_cleanup(decrypt_state.dcrypt);
}
+#ifdef CONFIG_CHUNKED_HASH
+ if (args->chunked_hashes) {
+ if (chunked_hash_state.chunk_data)
+ munmap(chunked_hash_state.chunk_data, chunked_hash_state.chunk_size);
+ }
+#endif
if (input_state.dgst) {
swupdate_HASH_cleanup(input_state.dgst);
}
diff --git a/include/util.h b/include/util.h
index 1703514269f1..13b5d0df6414 100644
--- a/include/util.h
+++ b/include/util.h
@@ -87,6 +87,8 @@ struct swupdate_copy {
uint32_t *checksum;
/* sw-description sha256 checksum */
unsigned char *hash;
+ /* juxtaposed sha256 checksums */
+ unsigned char *chunked_hashes;
/* encryption */
bool encrypted;
const char *imgivt;
--
2.39.2

[Dominique Martinet]

We don't need to nest ifs so much, chain steps as they become required.
The step order (input step, decryption, decompression) is preserved.

There is no intended functional change.

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---

core/cpio_utils.c | 33 ++++++++++++---------------------
1 file changed, 12 insertions(+), 21 deletions(-)

diff --git a/core/cpio_utils.c b/core/cpio_utils.c
index acb3f3b634f8..b0a97004baf6 100644
--- a/core/cpio_utils.c
+++ b/core/cpio_utils.c
@@ -616,31 +616,22 @@ int copyfile(struct swupdate_copy *args)
}
}

+ step = &input_step;
+ state = &input_state;
+
+ if (args->encrypted) {
+ decrypt_state.upstream_step = step;
+ decrypt_state.upstream_state = state;
+ step = &decrypt_step;
+ state = &decrypt_state;
+ }
+
#if defined(CONFIG_GUNZIP) || defined(CONFIG_ZSTD)
if (args->compressed) {
- if (args->encrypted) {
- decrypt_state.upstream_step = &input_step;
- decrypt_state.upstream_state = &input_state;
- decompress_state.upstream_step = &decrypt_step;
- decompress_state.upstream_state = &decrypt_state;
- } else {
- decompress_state.upstream_step = &input_step;
- decompress_state.upstream_state = &input_state;
- }
+ decompress_state.upstream_step = step;
+ decompress_state.upstream_state = state;
step = decompress_step;
state = &decompress_state;
- } else {
-#endif
- if (args->encrypted) {
- decrypt_state.upstream_step = &input_step;
- decrypt_state.upstream_state = &input_state;
- step = &decrypt_step;
- state = &decrypt_state;
- } else {
- step = &input_step;
- state = &input_state;
- }
-#if defined(CONFIG_GUNZIP) || defined(CONFIG_ZSTD)
}
#endif

--
2.39.2

[Dominique Martinet]

This avoids double-compuation of the checksum

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---

core/cpio_utils.c | 15 +++++++++------
1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/core/cpio_utils.c b/core/cpio_utils.c
index fd52c9ef0ba6..18cd474eaa8c 100644
--- a/core/cpio_utils.c
+++ b/core/cpio_utils.c
@@ -615,12 +615,6 @@ int copyfile(struct swupdate_copy *args)
if (args->checksum)
*args->checksum = 0;

- if (IsValidHash(args->hash)) {
- input_state.dgst = swupdate_HASH_init(SHA_DEFAULT);
- if (!input_state.dgst)
- return -EFAULT;
- }
-
if (args->encrypted) {
aes_key = get_aes_key();
if (args->imgivt && strlen(args->imgivt)) {
@@ -736,6 +730,9 @@ int copyfile(struct swupdate_copy *args)
close(tmpfd);
chunked_hash_state.chunked_hashes = args->chunked_hashes;

+ /* avoid computing sha256 twice */
+ args->hash = NULL;
+
chunked_hash_state.upstream_step = step;
chunked_hash_state.upstream_state = state;
step = &chunked_hash_step;
@@ -759,6 +756,12 @@ int copyfile(struct swupdate_copy *args)
}
#endif

+ if (IsValidHash(args->hash)) {
+ input_state.dgst = swupdate_HASH_init(SHA_DEFAULT);
+ if (!input_state.dgst)

+ return -EFAULT;
+ }
+

for (;;) {
ret = step(state, buffer, sizeof buffer);
if (ret < 0) {
--
2.39.2

[Dominique Martinet]

Having too many arguments make it difficult to see at a glance
what is passed where, and makes it hard to extend further.

Make copyfile() take a struct that describes all the current arguments
and fix all callers.

There is no intended functional change.

Signed-off-by: Dominique Martinet <dominique...@atmark-techno.com>
---

core/cpio_utils.c | 143 +++++++++++++++---------------------
core/installer.c | 19 +++--
core/stream_interface.c | 24 +++++-
handlers/copy_handler.c | 20 ++---
handlers/delta_handler.c | 29 +++++---
handlers/readback_handler.c | 20 ++---
include/util.h | 40 +++++++---
7 files changed, 155 insertions(+), 140 deletions(-)

diff --git a/core/cpio_utils.c b/core/cpio_utils.c
index a05a32cd59b5..acb3f3b634f8 100644
--- a/core/cpio_utils.c
+++ b/core/cpio_utils.c
@@ -466,9 +466,7 @@ static int hash_compare(struct swupdate_digest *dgst, unsigned char *hash)
return 0;
}

-static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nbytes, unsigned long *offs, unsigned long long seek,
- int skip_file, int __attribute__ ((__unused__)) compressed,
- uint32_t *checksum, unsigned char *hash, bool encrypted, const char *imgivt, writeimage callback)
+int copyfile(struct swupdate_copy *args)

{
unsigned int percent, prevpercent = 0;

int ret = 0;
@@ -478,12 +476,12 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
unsigned char ivtbuf[AES_BLK_SIZE];

struct InputState input_state = {
- .fdin = fdin,
+ .fdin = args->fdin,
.source = INPUT_FROM_FD,
.inbuf = NULL,
.pos = 0,
- .nbytes = nbytes,
- .offs = offs,
+ .nbytes = args->nbytes,
+ .offs = args->offs,
.dgst = NULL,
.checksum = 0
};
@@ -522,32 +520,33 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
/*
* If inbuf is set, read from buffer instead of from file
*/
- if (inbuf) {
- input_state.inbuf = inbuf;
+ if (args->inbuf) {
+ input_state.inbuf = args->inbuf;
input_state.source = INPUT_FROM_MEMORY;
}

PipelineStep step = NULL;
void *state = NULL;
uint8_t buffer[BUFF_SIZE];
+ writeimage callback = args->callback;

if (!callback) {
callback = copy_write;
}

- if (checksum)
- *checksum = 0;
+ if (args->checksum)
+ *args->checksum = 0;

- if (IsValidHash(hash)) {
+ if (IsValidHash(args->hash)) {
input_state.dgst = swupdate_HASH_init(SHA_DEFAULT);
if (!input_state.dgst)
return -EFAULT;
}

- if (encrypted) {
+ if (args->encrypted) {
aes_key = get_aes_key();
- if (imgivt && strlen(imgivt)) {
- if (!is_hex_str(imgivt) || ascii_to_bin(ivtbuf, sizeof(ivtbuf), imgivt)) {
+ if (args->imgivt && strlen(args->imgivt)) {
+ if (!is_hex_str(args->imgivt) || ascii_to_bin(ivtbuf, sizeof(ivtbuf), args->imgivt)) {
ERROR("Invalid image ivt");
return -EINVAL;
}
@@ -562,12 +561,12 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
}
}

- if (compressed) {
- if (compressed == COMPRESSED_TRUE) {
+ if (args->compressed) {
+ if (args->compressed == COMPRESSED_TRUE) {
WARN("compressed argument: boolean form is deprecated, use compressed = \"zlib\";");
}
#ifdef CONFIG_GUNZIP
- if (compressed == COMPRESSED_ZLIB || compressed == COMPRESSED_TRUE) {
+ if (args->compressed == COMPRESSED_ZLIB || args->compressed == COMPRESSED_TRUE) {
/*
* 16 + MAX_WBITS means that Zlib should expect and decode a
* gzip header.
@@ -583,7 +582,7 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
} else
#endif
#ifdef CONFIG_ZSTD
- if (compressed == COMPRESSED_ZSTD) {
+ if (args->compressed == COMPRESSED_ZSTD) {
if ((zstd_state.dctx = ZSTD_createDStream()) == NULL) {
ERROR("ZSTD_createDStream failed");
ret = -EFAULT;
@@ -595,22 +594,22 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
} else
#endif
{
- TRACE("Requested decompression method (%d) is not configured!", compressed);
+ TRACE("Requested decompression method (%d) is not configured!", args->compressed);
ret = -EINVAL;
goto copyfile_exit;
}
}

- if (seek) {
- int fdout = (out != NULL) ? *(int *)out : -1;
+ if (args->seek) {
+ int fdout = (args->out != NULL) ? *(int *)args->out : -1;
if (fdout < 0) {
ERROR("out argument: invalid fd or pointer");
ret = -EFAULT;
goto copyfile_exit;
}

- TRACE("offset has been defined: %llu bytes", seek);
- if (lseek(fdout, seek, SEEK_SET) < 0) {
+ TRACE("offset has been defined: %llu bytes", args->seek);
+ if (lseek(fdout, args->seek, SEEK_SET) < 0) {
ERROR("offset argument: seek failed");
ret = -EFAULT;
goto copyfile_exit;
@@ -618,8 +617,8 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
}

#if defined(CONFIG_GUNZIP) || defined(CONFIG_ZSTD)
- if (compressed) {
- if (encrypted) {
+ if (args->compressed) {
+ if (args->encrypted) {
decrypt_state.upstream_step = &input_step;
decrypt_state.upstream_state = &input_state;
decompress_state.upstream_step = &decrypt_step;
@@ -632,7 +631,7 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
state = &decompress_state;
} else {
#endif
- if (encrypted) {
+ if (args->encrypted) {
decrypt_state.upstream_step = &input_step;
decrypt_state.upstream_state = &input_state;
step = &decrypt_step;
@@ -653,7 +652,7 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
if (ret == 0) {
break;
}
- if (skip_file) {
+ if (args->skip_file) {
continue;
}
len = ret;
@@ -663,31 +662,32 @@ static int __swupdate_copy(int fdin, unsigned char *inbuf, void *out, size_t nby
* results corrupted. This lets the cleanup routine
* to remove it
*/
- if (callback(out, buffer, len) < 0) {
+ if (callback(args->out, buffer, len) < 0) {
ret = -ENOSPC;
goto copyfile_exit;
}

- percent = (unsigned)(100ULL * (nbytes - input_state.nbytes) / nbytes);
+ percent = (unsigned)(100ULL * (args->nbytes - input_state.nbytes) / args->nbytes);
if (percent != prevpercent) {
prevpercent = percent;
swupdate_progress_update(percent);
}
}

- if (IsValidHash(hash) && hash_compare(input_state.dgst, hash) < 0) {
+ if (IsValidHash(args->hash) && hash_compare(input_state.dgst, args->hash) < 0) {
ret = -EFAULT;
goto copyfile_exit;
}

- if (!inbuf) {
- ret = _fill_buffer(fdin, buffer, NPAD_BYTES(*offs), offs, checksum, NULL);
+ if (!args->inbuf) {
+ ret = _fill_buffer(args->fdin, buffer, NPAD_BYTES(*args->offs),
+ args->offs, args->checksum, NULL);
if (ret < 0)
DEBUG("Padding bytes are not read, ignoring");
}

- if (checksum != NULL) {
- *checksum = input_state.checksum;
+ if (args->checksum != NULL) {
+ *args->checksum = input_state.checksum;
}

ret = 0;
@@ -713,57 +713,23 @@ copyfile_exit:
return ret;
}

-int copyfile(int fdin, void *out, size_t nbytes, unsigned long *offs, unsigned long long seek,
- int skip_file, int __attribute__ ((__unused__)) compressed,
- uint32_t *checksum, unsigned char *hash, bool encrypted, const char *imgivt, writeimage callback)
-{
- return __swupdate_copy(fdin,
- NULL,
- out,
- nbytes,
- offs,
- seek,
- skip_file,
- compressed,
- checksum,
- hash,
- encrypted,
- imgivt,
- callback);
-}
-
-int copybuffer(unsigned char *inbuf, void *out, size_t nbytes, int __attribute__ ((__unused__)) compressed,
- unsigned char *hash, bool encrypted, const char *imgivt, writeimage callback)
-{
- return __swupdate_copy(-1,
- inbuf,
- out,
- nbytes,
- NULL,
- 0,
- 0,
- compressed,
- NULL,
- hash,
- encrypted,
- imgivt,
- callback);
-}
-

int copyimage(void *out, struct img_type *img, writeimage callback)

{
- return copyfile(img->fdin,
- out,

- img->size,
- (unsigned long *)&img->offset,
- img->seek,
- 0, /* no skip */
- img->compressed,
- &img->checksum,
- img->sha256,
- img->is_encrypted,
- img->ivt_ascii,

- callback);
+ struct swupdate_copy copy = {
+ .fdin = img->fdin,
+ .out = out,
+ .callback = callback,
+ .nbytes = img->size,
+ .offs = (unsigned long*)&img->offset,
+ .seek = img->seek,
+ .skip_file = 0,
+ .compressed = img->compressed,
+ .checksum = &img->checksum,
+ .hash = img->sha256,
+ .encrypted = img->is_encrypted,
+ .imgivt = img->ivt_ascii,
+ };
+ return copyfile(&copy);
}

int extract_cpio_header(int fd, struct filehdr *fhdr, unsigned long *offset)
@@ -846,8 +812,15 @@ int cpio_scan(int fd, struct swupdate_cfg *cfg, off_t start)
* use copyfile for checksum and hash verification, as we skip file
* we do not have to provide fdout
*/
- if (copyfile(fd, NULL, fdh.size, &offset, 0, 1, 0, &checksum, img ? img->sha256 : NULL,
- false, NULL, NULL) != 0) {
+ struct swupdate_copy copy = {
+ .fdin = fd,
+ .nbytes = fdh.size,
+ .offs = &offset,
+ .skip_file = 1,
+ .checksum = &checksum,
+ .hash = img ? img->sha256 : NULL,
+ };
+ if (copyfile(&copy) != 0) {
ERROR("invalid archive");
return -1;
}
diff --git a/core/installer.c b/core/installer.c
index cc5ca34b013d..0cb06b2ca419 100644
--- a/core/installer.c
+++ b/core/installer.c
@@ -141,13 +141,18 @@ static int extract_scripts(struct imglist *head)
return -ENOENT;
}

- ret = copyfile(fdin, &fdout, script->size, &offset, 0, 0,
- script->compressed,
- &checksum,
- script->sha256,
- script->is_encrypted,
- script->ivt_ascii,
- NULL);
+ struct swupdate_copy copy = {
+ .fdin = fdin,
+ .out = &fdout,
+ .nbytes = script->size,
+ .offs = &offset,
+ .compressed = script->compressed,
+ .checksum = &checksum,
+ .hash = script->sha256,
+ .encrypted = script->is_encrypted,
+ .imgivt = script->ivt_ascii,
+ };
+ ret = copyfile(&copy);
close(fdin);
close(fdout);

diff --git a/core/stream_interface.c b/core/stream_interface.c
index 5f3ad2e3fd93..14a483b19648 100644
--- a/core/stream_interface.c
+++ b/core/stream_interface.c
@@ -103,8 +103,15 @@ static int extract_file_to_tmp(int fd, const char *fname, unsigned long *poffs,
if (fdout < 0)
return -1;

- if (copyfile(fd, &fdout, fdh.size, poffs, 0, 0, 0, &checksum, NULL,
- encrypted, NULL, NULL) < 0) {
+ struct swupdate_copy copy = {
+ .fdin = fd,
+ .out = &fdout,
+ .nbytes = fdh.size,
+ .offs = poffs,
+ .checksum = &checksum,
+ .encrypted = encrypted,
+ };
+ if (copyfile(&copy) < 0) {
close(fdout);
return -1;
}
@@ -230,6 +237,13 @@ static int extract_files(int fd, struct swupdate_cfg *software)
fdout = -1;
offset = 0;

+ struct swupdate_copy copy = {
+ .fdin = fd,
+ .out = &fdout,
+ .nbytes = fdh.size,
+ .offs = &offset,
+ .checksum = &checksum,
+ };
/*
* If images are not streamed directly into the target
* copy them into TMPDIR to check if it is all ok
@@ -243,7 +257,8 @@ static int extract_files(int fd, struct swupdate_cfg *software)
close(fdout);
return -1;
}
- if (copyfile(fd, &fdout, fdh.size, &offset, 0, 0, 0, &checksum, img->sha256, false, NULL, NULL) < 0) {
+ copy.hash = img->sha256;
+ if (copyfile(&copy) < 0) {
close(fdout);
return -1;
}
@@ -255,7 +270,8 @@ static int extract_files(int fd, struct swupdate_cfg *software)
break;

case SKIP_FILE:
- if (copyfile(fd, &fdout, fdh.size, &offset, 0, skip, 0, &checksum, NULL, false, NULL, NULL) < 0) {
+ copy.skip_file = 1;
+ if (copyfile(&copy) < 0) {
return -1;
}
if (!swupdate_verify_chksum(checksum, &fdh)) {
diff --git a/handlers/copy_handler.c b/handlers/copy_handler.c
index 3e2f1f40f9e5..0dcb540a119e 100644
--- a/handlers/copy_handler.c
+++ b/handlers/copy_handler.c
@@ -121,18 +121,14 @@ static int copy_single_file(const char *path, ssize_t size, struct img_type *img
* Copying from device itself,
* no encryption or compression
*/
- ret = copyfile(fdin,
- &fdout,
- size,
- &offset,
- 0,

- 0, /* no skip */

- 0, /* no compressed */
- &checksum,
- 0, /* no sha256 */
- false, /* no encrypted */
- NULL, /* no IVT */
- NULL);
+ struct swupdate_copy copy = {
+ .fdin = fdin,
+ .out = &fdout,
+ .nbytes = size,
+ .offs = &offset,
+ .checksum = &checksum,
+ };
+ ret = copyfile(&copy);

close(fdout);
void *status;
diff --git a/handlers/delta_handler.c b/handlers/delta_handler.c
index 5ec243ee01d9..a14181f11d49 100644
--- a/handlers/delta_handler.c
+++ b/handlers/delta_handler.c
@@ -161,14 +161,14 @@ static int network_process_data(multipart_parser* p, const char *at, size_t leng
zck_get_chunk_number(priv->chunk),
priv->current.chunksize);
if (priv->current.chunksize != 0) {
- ret = copybuffer(priv->current.buf,
- &priv->fdout,
- priv->current.chunksize,
- COMPRESSED_ZSTD,
- hash,
- 0,
- NULL,
- NULL);
+ struct swupdate_copy copy = {
+ .inbuf = priv->current.buf,
+ .out = &priv->fdout,
+ .nbytes = priv->current.chunksize,
+ .compressed = COMPRESSED_ZSTD,
+ .hash = hash,
+ };
+ ret = copyfile(&copy);
} else
ret = 0; /* skipping, nothing to be copied */
/* Buffer can be discarged */
@@ -178,7 +178,7 @@ static int network_process_data(multipart_parser* p, const char *at, size_t leng
* if an error occurred, stops
*/
if (ret) {
- ERROR("copybuffer failed !");
+ ERROR("copyfile failed !");
priv->error_in_parser = true;
return -EFAULT;
}
@@ -845,8 +845,15 @@ static bool copy_existing_chunks(zckChunk **dstChunk, struct hnd_priv *priv)
zck_get_chunk_number(chunk),
start,
len);
- ret = copyfile(priv->fdsrc, &priv->fdout, len, &offset, 0, 0, COMPRESSED_FALSE,
- &checksum, hash, false, NULL, NULL);
+ struct swupdate_copy copy = {
+ .fdin = priv->fdsrc,
+ .out = &priv->fdout,
+ .nbytes = len,
+ .offs = &offset,
+ .checksum = &checksum,
+ .hash = hash,
+ };
+ ret = copyfile(&copy);

free(sha);
if (ret)
diff --git a/handlers/readback_handler.c b/handlers/readback_handler.c
index 70a82c1a893c..9e462eabe8e5 100644
--- a/handlers/readback_handler.c
+++ b/handlers/readback_handler.c
@@ -102,18 +102,14 @@ static int readback_postinst(struct img_type *img)
* the input device.
*/
unsigned long offset_out = 0;
- int status = copyfile(fdin,
- NULL, /* no output */
- size,
- &offset_out,
- 0, /* no output seek */
- 1, /* skip file, do not write to the output */
- 0, /* no compressed */
- NULL, /* no checksum */
- hash,
- false, /* no encrypted */
- NULL, /* no IVT */
- NULL); /* no callback */
+ struct swupdate_copy copy = {
+ .fdin = fdin,
+ .nbytes = size,
+ .offs = &offset_out,
+ .skip_file = 1,
+ .hash = hash,
+ };
+ int status = copyfile(&copy);
if (status == 0) {
INFO("Readback verification success");
} else {
diff --git a/include/util.h b/include/util.h
index 77da1b17ce15..1703514269f1 100644
--- a/include/util.h
+++ b/include/util.h
@@ -56,13 +56,42 @@ typedef enum {
SERVER_ID_REQUESTED,
} server_op_res_t;

-enum {
+enum compression_type {
COMPRESSED_FALSE,
COMPRESSED_TRUE,
COMPRESSED_ZLIB,
COMPRESSED_ZSTD,
};

+typedef int (*writeimage) (void *out, const void *buf, size_t len);
+
+struct swupdate_copy {
+ /* input: either fdin is set or fdin < 0 and inbuf */
+ int fdin;
+ unsigned char *inbuf;
+ /* data handler callback and output argument.
+ * out must point to a fd if seeking */
+ writeimage callback;
+ void *out;
+ /* amount of data to copy */
+ size_t nbytes;
+ /* pointer to offset within source, must be set for fd */
+ unsigned long *offs;
+ /* absolute offset to seek in output (*out) if non-zero */
+ unsigned long long seek;
+ /* skip callback: only verify input */
+ int skip_file;
+ /* decompression to use */
+ enum compression_type compressed;
+ /* cpio crc checksum */
+ uint32_t *checksum;
+ /* sw-description sha256 checksum */
+ unsigned char *hash;
+ /* encryption */
+ bool encrypted;
+ const char *imgivt;
+};
+
/*
* loglevel is used into TRACE / ERROR
* for values > LASTLOGLEVEL, it is an encoded field
@@ -188,8 +217,6 @@ bool strtobool(const char *s);
/*
* Function to extract / copy images
*/
-typedef int (*writeimage) (void *out, const void *buf, size_t len);
-
void *saferealloc(void *ptr, size_t size);
int copy_write(void *out, const void *buf, size_t len);
#if defined(__FreeBSD__)
@@ -200,13 +227,8 @@ int copy_write_padded(void *out, const void *buf, size_t len);
size_t
strlcpy(char *dst, const char * src, size_t size);
#endif
-int copyfile(int fdin, void *out, size_t nbytes, unsigned long *offs,
- unsigned long long seek,
- int skip_file, int compressed, uint32_t *checksum,
- unsigned char *hash, bool encrypted, const char *imgivt, writeimage callback);
+int copyfile(struct swupdate_copy *copy);
int copyimage(void *out, struct img_type *img, writeimage callback);
-int copybuffer(unsigned char *inbuf, void *out, size_t nbytes, int compressed,
- unsigned char *hash, bool encrypted, const char *imgivt, writeimage callback);
int openfileoutput(const char *filename);
int mkpath(char *dir, mode_t mode);
int swupdate_file_setnonblock(int fd, bool block);
--
2.39.2