On Tue, 27 Feb 2018 at 08:41, Gerd Hoffmann <kra...@redhat.com> wrote: > > From: Bandan Das <b...@redhat.com> > > Write of existing objects by the initiator is acheived by > making a temporary buffer with the new changes, deleting the > old file and then writing a new file with the same name. > > Also, add a "readonly" property which needs to be set to false > for deletion to work.
Hi -- Coverity points out a use-after-free here (CID 1399144): > +static int usb_mtp_deletefn(MTPState *s, MTPObject *o, uint32_t trans) > +{ > + MTPObject *iter, *iter2; > + bool partial_delete = false; > + bool success = false; > + > + /* > + * TODO: Add support for Protection Status > + */ > + > + QLIST_FOREACH(iter, &o->children, list) { > + if (iter->format == FMT_ASSOCIATION) { > + QLIST_FOREACH(iter2, &iter->children, list) { > + usb_mtp_deletefn(s, iter2, trans); > + } > + } > + } > + > + if (o->format == FMT_UNDEFINED_OBJECT) { > + if (remove(o->path)) { > + partial_delete = true; > + } else { > + usb_mtp_object_free_one(s, o); This call will call g_free(o)... > + success = true; > + } > + } > + > + if (o->format == FMT_ASSOCIATION) { ...but flow of control then falls through to here, where we try to access o->format. Preusumably this should be an "else if" clause ? > + if (rmdir(o->path)) { > + partial_delete = true; > + } else { > + usb_mtp_object_free_one(s, o); > + success = true; > + } > + } > + > + if (success && partial_delete) { > + return PARTIAL_DELETE; > + } > + if (!success && partial_delete) { > + return READ_ONLY; > + } > + return ALL_DELETE; > +} > + thanks -- PMM