[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PULL 3/5] usb-mtp: Support delete of mtp objects
From: |
Peter Maydell |
Subject: |
Re: [Qemu-devel] [PULL 3/5] usb-mtp: Support delete of mtp objects |
Date: |
Tue, 5 Mar 2019 16:14:34 +0000 |
On Tue, 27 Feb 2018 at 08:41, Gerd Hoffmann <address@hidden> wrote:
>
> From: Bandan Das <address@hidden>
>
> 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
[Prev in Thread] |
Current Thread |
[Next in Thread] |
- Re: [Qemu-devel] [PULL 3/5] usb-mtp: Support delete of mtp objects,
Peter Maydell <=