Re: [PATCH spice-server] reds: Check link header magic without waiting all header

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Frediano,

I'd replace "all" in subject with "for the whole"

On 01/26/2017 05:54 PM, Frediano Ziglio wrote:
This allows the connection to early fail in case initial bytes
are not correct.
This allows for instance VNC client to graceful fail connecting
to a spice-server. This happens easily as the two protocols
share the same range of ports.

Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
Tested-by: Daniel P. Berrange <berrange@xxxxxxxxxx>

Ack.


---

This is well described in
https://lists.freedesktop.org/archives/spice-devel/2017-January/035201.html
I'm not sure the comment I wrote is enough or I should copy some
explanation from the mail.

I think it's enough to add a reference to rhbz#1416692

Uri.


It add an extra read handling but I don't think it's really
a performance issue as happening only on initial connection.
---
 server/reds.c | 28 ++++++++++++++++++++--------
 1 file changed, 20 insertions(+), 8 deletions(-)

diff --git a/server/reds.c b/server/reds.c
index 29485a8..40c9485 100644
--- a/server/reds.c
+++ b/server/reds.c
@@ -2260,12 +2260,6 @@ static void reds_handle_read_header_done(void *opaque)
     header->minor_version = GUINT32_FROM_LE(header->minor_version);
     header->size = GUINT32_FROM_LE(header->size);

-    if (header->magic != SPICE_MAGIC) {
-        reds_send_link_error(link, SPICE_LINK_ERR_INVALID_MAGIC);
-        reds_link_free(link);
-        return;
-    }
-
     if (header->major_version != SPICE_VERSION_MAJOR) {
         if (header->major_version > 0) {
             reds_send_link_error(link, SPICE_LINK_ERR_VERSION_MISMATCH);
@@ -2292,13 +2286,31 @@ static void reds_handle_read_header_done(void *opaque)
                            link);
 }

+static void reds_handle_read_magic_done(void *opaque)
+{
+    RedLinkInfo *link = (RedLinkInfo *)opaque;
+    const SpiceLinkHeader *header = &link->link_header;
+
+    if (header->magic != SPICE_MAGIC) {
+        reds_send_link_error(link, SPICE_LINK_ERR_INVALID_MAGIC);
+        reds_link_free(link);
+        return;
+    }
+
+    reds_stream_async_read(link->stream,
+                           ((uint8_t *)&link->link_header) + sizeof(header->magic),
+                           sizeof(SpiceLinkHeader) - sizeof(header->magic),
+                           reds_handle_read_header_done,
+                           link);
+}
+
 static void reds_handle_new_link(RedLinkInfo *link)
 {
     reds_stream_set_async_error_handler(link->stream, reds_handle_link_error);
     reds_stream_async_read(link->stream,
                            (uint8_t *)&link->link_header,
-                           sizeof(SpiceLinkHeader),
-                           reds_handle_read_header_done,
+                           sizeof(link->link_header.magic),
+                           reds_handle_read_magic_done,
                            link);
 }



_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]