On 10/17/2016 01:29 PM, Frediano Ziglio wrote:
In both image_encoders_restore_glz_dictionary() and
image_encoders_get_glz_dictionary() shared-dict may
be NULL if size is too large, and the server gets
size from the network.
Both functions end up calling glz_enc_dictionary_create()
that calls glz_dictionary_window_create() where size is
checked.
Found by coverity.
Signed-off-by: Uri Lublin <uril@xxxxxxxxxx>
---
server/image-encoders.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/server/image-encoders.c b/server/image-encoders.c
index 39aca6c..9dfabd6 100644
--- a/server/image-encoders.c
+++ b/server/image-encoders.c
@@ -746,12 +746,13 @@ gboolean
image_encoders_get_glz_dictionary(ImageEncoders *enc,
shared_dict->refs++;
} else {
shared_dict = create_glz_dictionary(enc, client, id, window_size);
+ spice_return_val_if_fail(shared_dict != NULL, FALSE);
glz_dictionary_list = g_list_prepend(glz_dictionary_list,
shared_dict);
}
pthread_mutex_unlock(&glz_dictionary_list_lock);
enc->glz_dict = shared_dict;
- return shared_dict != NULL;
+ return TRUE;
}
static GlzSharedDictionary *restore_glz_dictionary(ImageEncoders *enc,
@@ -782,12 +783,13 @@ gboolean
image_encoders_restore_glz_dictionary(ImageEncoders *enc,
shared_dict->refs++;
} else {
shared_dict = restore_glz_dictionary(enc, client, id, restore_data);
+ spice_return_val_if_fail(shared_dict != NULL, FALSE);
glz_dictionary_list = g_list_prepend(glz_dictionary_list,
shared_dict);
}
pthread_mutex_unlock(&glz_dictionary_list_lock);
enc->glz_dict = shared_dict;
- return shared_dict != NULL;
+ return TRUE;
}
gboolean image_encoders_glz_create(ImageEncoders *enc, uint8_t id)
Note that you are creating dead locks.
Yes, your are right.
I'll send a replacement patch that fixes that.
Beside that is not clear what the change could cause to the
upper layer.
Upper layer should already handle a case where these functions
return NULL.
I think the actual logic is supposing that dictionary creation (like
memory allocation) is not failing.
Since it depends on a value that comes from the client that
assumption should not be made. Alternatively we
can specifically check the number early
Thanks,
Uri.
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel