Message ID | 20230707204319.1297224-1-nick.potenski@garmin.com |
---|---|
State | Changes Requested |
Delegated to: | Stefano Babic |
Headers | show |
Series | [v2] Fix calculation and reading of end-of-file padding | expand |
Hi Nick, this is not applied due to issue (just return code from fcntl) reported by coverity, see previous e-mail on ML. Can you fix them and repost ? Thanks ! Best regards, Stefano Babic On 07.07.23 22:43, 'Nick Potenski' via swupdate wrote: > The stream interface may not read all data from the socket which could > cause swupdate to close the socket fd before a client has written all > data, resulting in an EPIPE on the client. > > Instead of trying to calculate and expect a specific number of padding > bytes, perform best effort to read up to 512 bytes or until an error or > timeout is encountered. > > Signed-off-by: Nick Potenski <nick.potenski@garmin.com> > --- > core/cpio_utils.c | 35 +++++++++++++++++++++++++++++------ > core/stream_interface.c | 2 +- > include/cpiohdr.h | 2 +- > 3 files changed, 31 insertions(+), 8 deletions(-) > > diff --git a/core/cpio_utils.c b/core/cpio_utils.c > index bcf4818..1b4f2b8 100644 > --- a/core/cpio_utils.c > +++ b/core/cpio_utils.c > @@ -11,6 +11,8 @@ > #include <stdio.h> > #include <unistd.h> > #include <errno.h> > +#include <fcntl.h> > +#include <poll.h> > #ifdef CONFIG_GUNZIP > #include <zlib.h> > #endif > @@ -96,25 +98,46 @@ static int fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigned > * Read padding that could exists between the cpio trailer and the end-of-file. > * cpio aligns the file to 512 bytes > */ > -void extract_padding(int fd, unsigned long *offset) > +void extract_padding(int fd) > { > int padding; > ssize_t len; > unsigned char buf[512]; > + int old_flags; > + struct pollfd pfd; > + int retval; > > - if (fd < 0 || !offset) > + if (fd < 0) > return; > > - padding = (512 - (*offset % 512)) % 512; > - if (padding) { > - TRACE("Expecting %d padding bytes at end-of-file", padding); > + old_flags = fcntl(fd, F_GETFL); > + if (old_flags < 0) > + return; > + fcntl(fd, F_SETFL, old_flags | O_NONBLOCK); > + > + pfd.fd = fd; > + pfd.events = POLLIN; > + > + padding = 512; > + > + TRACE("Expecting up to 512 padding bytes at end-of-file"); > + do { > + retval = poll(&pfd, 1, 1000); > + if (retval < 0) { > + DEBUG("Failure while waiting on fd %d: %s", fd, strerror(errno)); > + fcntl(fd, F_SETFL, old_flags); > + return; > + } > len = read(fd, buf, padding); > if (len < 0) { > DEBUG("Failure while reading padding %d: %s", fd, strerror(errno)); > + fcntl(fd, F_SETFL, old_flags); > return; > } > - } > + padding -= len; > + } while (len > 0 && padding > 0); > > + fcntl(fd, F_SETFL, old_flags); > return; > } > > diff --git a/core/stream_interface.c b/core/stream_interface.c > index 34435fa..3f0b952 100644 > --- a/core/stream_interface.c > +++ b/core/stream_interface.c > @@ -205,7 +205,7 @@ static int extract_files(int fd, struct swupdate_cfg *software) > * to 512 bytes from the socket until the > * client stops writing > */ > - extract_padding(fd, &offset); > + extract_padding(fd); > status = STREAM_END; > break; > } > diff --git a/include/cpiohdr.h b/include/cpiohdr.h > index d263776..0be844a 100644 > --- a/include/cpiohdr.h > +++ b/include/cpiohdr.h > @@ -59,7 +59,7 @@ struct filehdr { > int get_cpiohdr(unsigned char *buf, struct filehdr *fhdr); > 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); > -void extract_padding(int fd, unsigned long *offset); > +void extract_padding(int fd); > bool swupdate_verify_chksum(const uint32_t chk1, struct filehdr *fhdr); > > #endif
diff --git a/core/cpio_utils.c b/core/cpio_utils.c index bcf4818..1b4f2b8 100644 --- a/core/cpio_utils.c +++ b/core/cpio_utils.c @@ -11,6 +11,8 @@ #include <stdio.h> #include <unistd.h> #include <errno.h> +#include <fcntl.h> +#include <poll.h> #ifdef CONFIG_GUNZIP #include <zlib.h> #endif @@ -96,25 +98,46 @@ static int fill_buffer(int fd, unsigned char *buf, unsigned int nbytes, unsigned * Read padding that could exists between the cpio trailer and the end-of-file. * cpio aligns the file to 512 bytes */ -void extract_padding(int fd, unsigned long *offset) +void extract_padding(int fd) { int padding; ssize_t len; unsigned char buf[512]; + int old_flags; + struct pollfd pfd; + int retval; - if (fd < 0 || !offset) + if (fd < 0) return; - padding = (512 - (*offset % 512)) % 512; - if (padding) { - TRACE("Expecting %d padding bytes at end-of-file", padding); + old_flags = fcntl(fd, F_GETFL); + if (old_flags < 0) + return; + fcntl(fd, F_SETFL, old_flags | O_NONBLOCK); + + pfd.fd = fd; + pfd.events = POLLIN; + + padding = 512; + + TRACE("Expecting up to 512 padding bytes at end-of-file"); + do { + retval = poll(&pfd, 1, 1000); + if (retval < 0) { + DEBUG("Failure while waiting on fd %d: %s", fd, strerror(errno)); + fcntl(fd, F_SETFL, old_flags); + return; + } len = read(fd, buf, padding); if (len < 0) { DEBUG("Failure while reading padding %d: %s", fd, strerror(errno)); + fcntl(fd, F_SETFL, old_flags); return; } - } + padding -= len; + } while (len > 0 && padding > 0); + fcntl(fd, F_SETFL, old_flags); return; } diff --git a/core/stream_interface.c b/core/stream_interface.c index 34435fa..3f0b952 100644 --- a/core/stream_interface.c +++ b/core/stream_interface.c @@ -205,7 +205,7 @@ static int extract_files(int fd, struct swupdate_cfg *software) * to 512 bytes from the socket until the * client stops writing */ - extract_padding(fd, &offset); + extract_padding(fd); status = STREAM_END; break; } diff --git a/include/cpiohdr.h b/include/cpiohdr.h index d263776..0be844a 100644 --- a/include/cpiohdr.h +++ b/include/cpiohdr.h @@ -59,7 +59,7 @@ struct filehdr { int get_cpiohdr(unsigned char *buf, struct filehdr *fhdr); 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); -void extract_padding(int fd, unsigned long *offset); +void extract_padding(int fd); bool swupdate_verify_chksum(const uint32_t chk1, struct filehdr *fhdr); #endif
The stream interface may not read all data from the socket which could cause swupdate to close the socket fd before a client has written all data, resulting in an EPIPE on the client. Instead of trying to calculate and expect a specific number of padding bytes, perform best effort to read up to 512 bytes or until an error or timeout is encountered. Signed-off-by: Nick Potenski <nick.potenski@garmin.com> --- core/cpio_utils.c | 35 +++++++++++++++++++++++++++++------ core/stream_interface.c | 2 +- include/cpiohdr.h | 2 +- 3 files changed, 31 insertions(+), 8 deletions(-)