Peter Maydell <peter.mayd...@linaro.org> writes: > On Wed, 30 Jan 2019 at 07:41, Gerd Hoffmann <kra...@redhat.com> wrote: >> >> From: Bandan Das <b...@redhat.com> >> >> For every MTP_WRITE_BUF_SZ copied, this patch writes it to file before >> getting the next block of data. The file is kept opened for the >> duration of the operation but the sanity checks on the write operation >> are performed only once when the write operation starts. Additionally, >> we also update the file size in the object metadata once the file has >> completely been written. >> >> Suggested-by: Gerd Hoffman <kra...@redhat.com> >> Signed-off-by: Bandan Das <b...@redhat.com> >> Message-id: 20190129131908.27924-3-...@redhat.com >> Signed-off-by: Gerd Hoffmann <kra...@redhat.com> > > Hi; Coverity has spotted a couple of issues with this patch: > > >> +static void usb_mtp_update_object(MTPObject *parent, char *name) >> +{ >> + MTPObject *o = >> + usb_mtp_object_lookup_name(parent, name, strlen(name)); >> + >> + if (o) { >> + lstat(o->path, &o->stat); > > CID 1398651: We don't check the return value of this lstat() for failure. >
Thanks, will post a patch for this. >> + } >> +} >> + >> static void usb_mtp_write_data(MTPState *s) >> { >> MTPData *d = s->data_out; > > [...] > >> + case WRITE_CONTINUE: >> + case WRITE_END: >> + rc = write_retry(d->fd, d->data, d->data_offset, >> + d->offset - d->data_offset); >> + if (rc != d->data_offset) { >> usb_mtp_queue_result(s, RES_STORE_FULL, d->trans, >> 0, 0, 0, 0); >> goto done; >> + } >> + if (d->write_status != WRITE_END) { >> + return; > > CID 1398642: This early-return case in usb_mtp_write_data() returns > from the function without doing any of the cleanup (closing file, > freeing data, etc). Possibly it should be "goto done;" instead ? > The specific thing Coverity complains about is the memory pointed > to by "path". > I believe this is a false positive, there's still more data incoming and we have successfully written the data we got this time, so we return without freeing up any of the structures. I will add a comment here. Bandan > thanks > -- PMM