[Spice-devel] [vdagent-linux] x11: More udscs_read_callback memory ownership issues

[Pavel Grunt]

On Thu, 2016-11-24 at 16:57 +0100, Christophe Fergeau wrote:
> This time it's vdagent_x11_clipboard_data() which is called from
> daemon_read_complete() which was missed and which was still freeing
> data
> it no longer owns.
> 
> This fixes https://bugs.freedesktop.org/show_bug.cgi?id=98830
Ack

> ---
>  src/vdagent/vdagent.c |  2 --
>  src/vdagent/x11.c     | 22 +++++++++++-----------
>  2 files changed, 11 insertions(+), 13 deletions(-)
> 
> diff --git a/src/vdagent/vdagent.c b/src/vdagent/vdagent.c
> index 085b84a..3d195b1 100644
> --- a/src/vdagent/vdagent.c
> +++ b/src/vdagent/vdagent.c
> @@ -72,8 +72,6 @@ static void daemon_read_complete(struct
> udscs_connection **connp,
>      case VDAGENTD_CLIPBOARD_DATA:
>          vdagent_x11_clipboard_data(x11, header->arg1, header->arg2,
>                                     data, header->size);
> -        /* vdagent_x11_clipboard_data takes ownership of the data
> (or frees
> -           it immediately) */
>          break;
>      case VDAGENTD_CLIPBOARD_RELEASE:
>          vdagent_x11_clipboard_release(x11, header->arg1);
> diff --git a/src/vdagent/x11.c b/src/vdagent/x11.c
> index e88a8ea..4dd1aa8 100644
> --- a/src/vdagent/x11.c
> +++ b/src/vdagent/x11.c
> @@ -1216,7 +1216,6 @@ void vdagent_x11_clipboard_data(struct
> vdagent_x11 *x11, uint8_t selection,
>              SELPRINTF("received clipboard data while still sending"
>                        " data from previous request, ignoring");
>          }
> -        free(data);
>          return;
>      }
>  
> @@ -1225,7 +1224,6 @@ void vdagent_x11_clipboard_data(struct
> vdagent_x11 *x11, uint8_t selection,
>              SELPRINTF("received clipboard data without an "
>                        "outstanding selection request, ignoring");
>          }
> -        free(data);
>          return;
>      }
>  
> @@ -1244,7 +1242,6 @@ void vdagent_x11_clipboard_data(struct
> vdagent_x11 *x11, uint8_t selection,
>                        type_from_event, type);
>          }
>          vdagent_x11_send_selection_notify(x11, None, NULL);
> -        free(data);
>  
>          /* Flush output buffers and consume any pending events */
>          vdagent_x11_do_read(x11);
> @@ -1266,14 +1263,19 @@ void vdagent_x11_clipboard_data(struct
> vdagent_x11 *x11, uint8_t selection,
>                          x11->incr_atom, 32, PropModeReplace,
>                          (unsigned char*)&len, 1);
>          if (vdagent_x11_restore_error_handler(x11) == 0) {
> -            x11->selection_req_data = data;
> -            x11->selection_req_data_pos = 0;
> -            x11->selection_req_data_size = size;
> -            x11->selection_req_atom = prop;
> -            vdagent_x11_send_selection_notify(x11, prop, x11-
> >selection_req);
> +            /* duplicate data */
> +            x11->selection_req_data = malloc(size);
> +            if (x11->selection_req_data != NULL) {
> +                memcpy(x11->selection_req_data, data, size);
> +                x11->selection_req_data_pos = 0;
> +                x11->selection_req_data_size = size;
> +                x11->selection_req_atom = prop;
> +                vdagent_x11_send_selection_notify(x11, prop, x11-
> >selection_req);
> +            } else {
> +                SELPRINTF("out of memory allocating selection
> buffer");
> +            }
>          } else {
>              SELPRINTF("clipboard data sent failed, requestor window
> gone");
> -            free(data);
>          }
>      } else {
>          vdagent_x11_set_error_handler(x11,
> vdagent_x11_ignore_bad_window_handler);
> @@ -1284,8 +1286,6 @@ void vdagent_x11_clipboard_data(struct
> vdagent_x11 *x11, uint8_t selection,
>              vdagent_x11_send_selection_notify(x11, prop, NULL);
>          else
>              SELPRINTF("clipboard data sent failed, requestor window
> gone");
> -
> -        free(data);
>      }
>  
>      /* Flush output buffers and consume any pending events */