Hi,
On 03/13/2012 02:40 PM, Daniel P. Berrange wrote:
From: "Daniel P. Berrange"<berra...@redhat.com>
A few functions have very large arrays declared on the stack.
Replace these with heap allocations, to reduce risk of stack
overflows in deep callpaths
---
gtk/channel-playback.c | 6 ++++--
gtk/spice-channel.c | 16 ++++++++++++----
2 files changed, 16 insertions(+), 6 deletions(-)
diff --git a/gtk/channel-playback.c b/gtk/channel-playback.c
index 32f8b1a..c353cd2 100644
--- a/gtk/channel-playback.c
+++ b/gtk/channel-playback.c
@@ -353,10 +353,12 @@ static void playback_handle_data(SpiceChannel *channel,
SpiceMsgIn *in)
packet->data, packet->data_size);
break;
case SPICE_AUDIO_DATA_MODE_CELT_0_5_1: {
- celt_int16_t pcm[256 * 2];
+ celt_int16_t *pcm;
+ gsize pcmLen = 256 * 2;
g_return_if_fail(c->celt_decoder != NULL);
+ pcm = g_new0(celt_int16_t, pcmLen);
if (celt051_decode(c->celt_decoder, packet->data,
packet->data_size, pcm) != CELT_OK) {
g_warning("celt_decode() error");
@@ -364,7 +366,7 @@ static void playback_handle_data(SpiceChannel *channel,
SpiceMsgIn *in)
}
emit_main_context(channel, SPICE_PLAYBACK_DATA,
- (uint8_t *)pcm, sizeof(pcm));
+ (uint8_t *)pcm, pcmLen);
break;
}
default:
You seem to be never freeing the data here. Also I disagree with switching
to malloc / free here in general. This path will get called many times a second,
malloc / free is not a cheap operation and 1024 bytes is not such a big amount
to
put on the stack.
diff --git a/gtk/spice-channel.c b/gtk/spice-channel.c
index cdc86ba..248f387 100644
--- a/gtk/spice-channel.c
+++ b/gtk/spice-channel.c
@@ -941,17 +941,24 @@ static int spice_channel_read_sasl(SpiceChannel *channel,
void *data, size_t len
/* c->sasl_decoded_length, c->sasl_decoded_offset); */
if (c->sasl_decoded == NULL || c->sasl_decoded_length == 0) {
- char encoded[8192]; /* should stay lower than maxbufsize */
+ char *encoded;
+ gsize encodedLen;
int err, ret;
+ encodedLen = 8192;
+ encoded = g_new0(char, encodedLen);
+
g_warn_if_fail(c->sasl_decoded_offset == 0);
- ret = spice_channel_read_wire(channel, encoded, sizeof(encoded));
- if (ret< 0)
+ ret = spice_channel_read_wire(channel, encoded, encodedLen);
+ if (ret< 0) {
+ g_free(encoded);
return ret;
+ }
err = sasl_decode(c->sasl_conn, encoded, ret,
&c->sasl_decoded,&c->sasl_decoded_length);
+ g_free(encoded);
if (err != SASL_OK) {
g_warning("Failed to decode SASL data %s",
sasl_errstring(err, NULL, NULL));
This one looks fine from the free pov. But again this is a semi hot
code path, and malloc / free is not a cheap operation, also there are
no guarantees wrt cache hotness when it comes to the heap, where as the stack
is likely hot in the cache.
@@ -1629,6 +1636,7 @@ static void spice_channel_recv_link_msg(SpiceChannel
*channel)
{
SpiceChannelPrivate *c;
int rc, num_caps, i;
+ uint32_t *caps;
g_return_if_fail(channel != NULL);
g_return_if_fail(channel->priv != NULL);
@@ -1666,7 +1674,7 @@ static void spice_channel_recv_link_msg(SpiceChannel
*channel)
/* see original spice/client code: */
/* g_return_if_fail(c->peer_msg + c->peer_msg->caps_offset * sizeof(uint32_t)>
c->peer_msg + c->peer_hdr.size); */
- uint32_t *caps = (uint32_t *)((uint8_t *)c->peer_msg +
c->peer_msg->caps_offset);
+ caps = (uint32_t *)((uint8_t *)c->peer_msg + c->peer_msg->caps_offset);
g_array_set_size(c->remote_common_caps, c->peer_msg->num_common_caps);
for (i = 0; i< c->peer_msg->num_common_caps; i++, caps++) {
This seems an unrelated fix.
Regards,
Hans
_______________________________________________
Spice-devel mailing list
Spice-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/spice-devel
