diff mbox series

R: [PATCH] stream_interface: Keep reading the cpio padding, if any, up to 512 bytes from the socket until the client stops writing.

Message ID AM6PR08MB316001753544269A073E6A0C81120@AM6PR08MB3160.eurprd08.prod.outlook.com
State Changes Requested
Headers show
Series R: [PATCH] stream_interface: Keep reading the cpio padding, if any, up to 512 bytes from the socket until the client stops writing. | expand

Commit Message

Luca Pesce Feb. 21, 2020, 1:44 p.m. UTC 
This patch is also related to these previous discussions, describing the same issue:

https://groups.google.com/forum/#!searchin/swupdate/cpio$20padding|sort:date/swupdate/D3QvZn5ePWg/6_bgyy-pEAAJ
https://groups.google.com/forum/#!searchin/swupdate/hit$20a$20wall%7Csort:date/swupdate/Vilq9qMgVgY/FdE03JdWBAAJ

The patch has been tested against regular cpio archives (512-bytes aligned, with trailing zero-padding) and also against archives with padding manually removed (the later because we saw in the ML that somebody was doing that in yocto to work around the issue in swupdate).
The issue is really related to timings: scheduling between the two processes, network bandwidth, pipe buffering, etc... So, a quick way to reproduce the issue is to artificially slow down the streaming client writings to the IPC socket (can be suricatta or the 'client' example application), thus simulating a low bandwidth situation and making the scenario more likely to happen. Another way is to just add even more zeros at the EOF.
diff mbox series

Patch

diff --git a/core/cpio_utils.c b/core/cpio_utils.c
index 03776e2..9282996 100644
--- a/core/cpio_utils.c
+++ b/core/cpio_utils.c
@@ -79,6 +79,36 @@  static int fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigned
        return count;
 }

+/*
+ * Read padding that could exists between the cpio trailer and the end-of-file.
+ * cpio aligns the file to 512 bytes
+ */
+int extract_padding(int fd, unsigned long *offset)
+{
+    int padding;
+    ssize_t len;
+       unsigned char buf[512];
+
+    if (fd < 0 || !offset)
+        return 0;
+
+    padding = (512 - (*offset % 512)) % 512;
+    if (padding) {
+        TRACE("Expecting %d padding bytes at end-of-file", padding);
+        len = read(fd, buf, padding);
+        if (len < 0) {
+            ERROR("Failure while reading padding %d: %s", fd, strerror(errno));
+            return 0;
+        }
+        if (len == 0) {
+            /* No padding found */
+            return 0;
+        }
+    }
+
+    return 0;
+}
+
 /*
  * Export the copy_write{,_*} functions to be used in other modules
  * for copying a buffer to a file.
@@ -760,7 +790,6 @@  int cpio_scan(int fd, struct swupdate_cfg *cfg, off_t start)
        int file_listed;
        uint32_t checksum;

-
        while (1) {
                file_listed = 0;
                start = offset;
diff --git a/corelib/stream_interface.c b/corelib/stream_interface.c
index ff2f6d0..d97a74a 100644
--- a/corelib/stream_interface.c
+++ b/corelib/stream_interface.c
@@ -174,6 +174,9 @@  static int extract_files(int fd, struct swupdate_cfg *software)
                                return -1;
                        }
                        if (strcmp("TRAILER!!!", fdh.filename) == 0) {
+                /* Keep reading the cpio padding, if any, up to 512 bytes from
+                 * the socket until the client stops writing */
+                extract_padding(fd, &offset);
                                status = STREAM_END;
                                break;
                        }
diff --git a/include/cpiohdr.h b/include/cpiohdr.h
index 855d5e8..9ef0109 100644
--- a/include/cpiohdr.h
+++ b/include/cpiohdr.h
@@ -53,5 +53,6 @@  int get_cpiohdr(unsigned char *buf, unsigned long *size,
                        unsigned long *namesize, unsigned long *chksum);
 int extract_cpio_header(int fd, struct filehdr *fhdr, unsigned long *offset);
 int extract_img_from_cpio(int fd, unsigned long offset, struct filehdr *fdh);
+int extract_padding(int fd, unsigned long *offset);

 #endif