On Wed, 2015-12-09 at 13:36 +0000, Frediano Ziglio wrote:
> During display_channel_handle_migrate_data the pointer is passed
> to different functions which could release it making the pointer
> invalid. Make sure pointer is not freed while processing.
Maybe I didn't go down every path, but I didn't see any cases where the channel
client is freed during processing. But regardless, I'd say it's a good idea to
maintain a reference during processing.
Acked-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
>
> Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
> ---
> server/display-channel.c | 8 ++++++--
> server/red-channel.c | 6 ++----
> server/red-channel.h | 2 ++
> 3 files changed, 10 insertions(+), 6 deletions(-)
>
> diff --git a/server/display-channel.c b/server/display-channel.c
> index 6b1043b..fe05121 100644
> --- a/server/display-channel.c
> +++ b/server/display-channel.c
> @@ -1204,6 +1204,7 @@ int display_channel_handle_migrate_data(DisplayChannel
> *display)
> uint64_t end_time = red_get_monotonic_time() +
> DISPLAY_CLIENT_MIGRATE_DATA_TIMEOUT;
> RedChannel *channel = &display->common.base;
> RedChannelClient *rcc;
> + int ret = FALSE;
>
> if (!red_channel_is_waiting_for_migrate_data(&display->common.base)) {
> return FALSE;
> @@ -1214,6 +1215,7 @@ int display_channel_handle_migrate_data(DisplayChannel
> *display)
>
> rcc = SPICE_CONTAINEROF(ring_get_head(&channel->clients),
> RedChannelClient, channel_link);
>
> + red_channel_client_ref(rcc);
> for (;;) {
> red_channel_client_receive(rcc);
> if (!red_channel_client_is_connected(rcc)) {
> @@ -1221,7 +1223,8 @@ int display_channel_handle_migrate_data(DisplayChannel
> *display)
> }
>
> if (!red_channel_client_is_waiting_for_migrate_data(rcc)) {
> - return TRUE;
> + ret = TRUE;
> + break;
> }
> if (red_get_monotonic_time() > end_time) {
> spice_warning("timeout");
> @@ -1230,7 +1233,8 @@ int display_channel_handle_migrate_data(DisplayChannel
> *display)
> }
> usleep(DISPLAY_CLIENT_RETRY_INTERVAL);
> }
> - return FALSE;
> + red_channel_client_unref(rcc);
> + return ret;
> }
>
> void display_channel_flush_all_surfaces(DisplayChannel *display)
> diff --git a/server/red-channel.c b/server/red-channel.c
> index 4b1d426..7dfb9bf 100644
> --- a/server/red-channel.c
> +++ b/server/red-channel.c
> @@ -116,8 +116,6 @@ static inline int
> red_channel_client_waiting_for_ack(RedChannelClient *rcc);
> */
> static void red_channel_ref(RedChannel *channel);
> static void red_channel_unref(RedChannel *channel);
> -static void red_channel_client_ref(RedChannelClient *rcc);
> -static void red_channel_client_unref(RedChannelClient *rcc);
>
> static uint32_t full_header_get_msg_size(SpiceDataHeaderOpaque *header)
> {
> @@ -1231,12 +1229,12 @@ static void red_channel_unref(RedChannel *channel)
> }
> }
>
> -static void red_channel_client_ref(RedChannelClient *rcc)
> +void red_channel_client_ref(RedChannelClient *rcc)
> {
> rcc->refs++;
> }
>
> -static void red_channel_client_unref(RedChannelClient *rcc)
> +void red_channel_client_unref(RedChannelClient *rcc)
> {
> if (!--rcc->refs) {
> spice_debug("destroy rcc=%p", rcc);
> diff --git a/server/red-channel.h b/server/red-channel.h
> index 8206cec..2058350 100644
> --- a/server/red-channel.h
> +++ b/server/red-channel.h
> @@ -418,6 +418,8 @@ int red_channel_is_waiting_for_migrate_data(RedChannel
> *channel);
>
> void red_channel_client_destroy(RedChannelClient *rcc);
> void red_channel_destroy(RedChannel *channel);
> +void red_channel_client_ref(RedChannelClient *rcc);
> +void red_channel_client_unref(RedChannelClient *rcc);
>
> int red_channel_client_test_remote_common_cap(RedChannelClient *rcc, uint32_t
> cap);
> int red_channel_client_test_remote_cap(RedChannelClient *rcc, uint32_t cap);
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel