| Message ID | 1536669899-6708-2-git-send-email-mark.jonas@de.bosch.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | cpio_utils: fix infinite read when interrupting an update | expand |
Hi Mark, Ben, On 11/09/2018 14:44, Mark Jonas wrote: > From: Ben Guan <ben.guan@cn.bosch.com> > > When using the IPC API to do an update from a stream, a new update > cannot be started if the previous one is interrupted. > > The root cause is: when the client closes the socket, the read on the > server side will return 0 and it is treated as a successful read. Thus > it goes into an infinite read loop. > > It can be reproduced: > 1. Start the server via "swupdate". > 2. Trigger an update via "swupdate-client image.swu". > 3. Cancel the update via Ctrl + C. > 4. Trigger another update via "swupdate-client image.swu". It could be but I can't reproduce the issue. I added this to be sure to go into the case: diff --git a/core/cpio_utils.c b/core/cpio_utils.c index 40ae474..598223f 100644 --- a/core/cpio_utils.c +++ b/core/cpio_utils.c @@ -135,6 +135,8 @@ static int input_step(void *state, void *buffer, size_t size) size = s->nbytes; } int ret = fill_buffer(s->fdin, buffer, size, s->offs, &s->checksum, s->dgst); + if (!ret) + ERROR("INPUT STATE 0"); if (ret < 0) { return ret; } and when I stop the client I see on the log: [ERROR] : SWUPDATE failed [0] ERROR core/cpio_utils.c : input_step : 139 : INPUT STATE 0 [TRACE] : SWUPDATE running : [network_initializer] : Valid image found: copying to FLASH [INFO ] : SWUPDATE running : Installation in progress [TRACE] : SWUPDATE running : [install_single_image] : Found installer for stream update-image-twister-20170521145947.swu swuforward [DEBUG] : SWUPDATE running : [channel_set_options] : cURL's low download speed timeout is disabled, this is most probably not what you want. Adapted it to 300s instead. but SWUpdate goes further and then stops with error as expected. I cannot see any endless loop. Can you maybe try to explain how to get in trouble ? Even if the ret = 0 is not recognizes, the buffer is consumed because the size is increased - SWUpdate then stops because the checksum is obviously not verified. > > Signed-off-by: Ben Guan <ben.guan@cn.bosch.com> > Signed-off-by: Mark Jonas <mark.jonas@de.bosch.com> > --- > core/cpio_utils.c | 8 ++++---- > 1 file changed, 4 insertions(+), 4 deletions(-) > > diff --git a/core/cpio_utils.c b/core/cpio_utils.c > index 40ae474..50123eb 100644 > --- a/core/cpio_utils.c > +++ b/core/cpio_utils.c > @@ -135,7 +135,7 @@ static int input_step(void *state, void *buffer, size_t size) > size = s->nbytes; > } > int ret = fill_buffer(s->fdin, buffer, size, s->offs, &s->checksum, s->dgst); > - if (ret < 0) { > + if (ret <= 0) { > return ret; > } > s->nbytes -= size; > @@ -171,7 +171,7 @@ static int decrypt_step(void *state, void *buffer, size_t size) > } > > ret = s->upstream_step(s->upstream_state, s->input, sizeof s->input); > - if (ret < 0) { > + if (ret <= 0) { > return ret; > } > > @@ -232,7 +232,7 @@ static int gunzip_step(void *state, void *buffer, size_t size) > while (outlen == 0) { > if (s->strm.avail_in == 0) { > ret = s->upstream_step(s->upstream_state, s->input, sizeof s->input); > - if (ret < 0) { > + if (ret <= 0) { > return ret; > } > s->strm.avail_in = ret; > @@ -317,7 +317,7 @@ int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsi > if (!input_state.dgst) > return -EFAULT; > } > - > + > if (encrypted) { > aes_key = get_aes_key(); > ivt = get_aes_ivt(); > Best regards, Stefano Babic
diff --git a/core/cpio_utils.c b/core/cpio_utils.c index 40ae474..50123eb 100644 --- a/core/cpio_utils.c +++ b/core/cpio_utils.c @@ -135,7 +135,7 @@ static int input_step(void *state, void *buffer, size_t size) size = s->nbytes; } int ret = fill_buffer(s->fdin, buffer, size, s->offs, &s->checksum, s->dgst); - if (ret < 0) { + if (ret <= 0) { return ret; } s->nbytes -= size; @@ -171,7 +171,7 @@ static int decrypt_step(void *state, void *buffer, size_t size) } ret = s->upstream_step(s->upstream_state, s->input, sizeof s->input); - if (ret < 0) { + if (ret <= 0) { return ret; } @@ -232,7 +232,7 @@ static int gunzip_step(void *state, void *buffer, size_t size) while (outlen == 0) { if (s->strm.avail_in == 0) { ret = s->upstream_step(s->upstream_state, s->input, sizeof s->input); - if (ret < 0) { + if (ret <= 0) { return ret; } s->strm.avail_in = ret; @@ -317,7 +317,7 @@ int copyfile(int fdin, void *out, unsigned int nbytes, unsigned long *offs, unsi if (!input_state.dgst) return -EFAULT; } - + if (encrypted) { aes_key = get_aes_key(); ivt = get_aes_ivt();