On Fri, Mar 25, 2011 at 03:07:07PM +0100, Hans de Goede wrote: > Hi, > > Thanks for the review! >
And thanks for the clarification, corrected on both cases, so: ACK series. > On 03/25/2011 02:52 PM, Alon Levy wrote: > >On Thu, Mar 24, 2011 at 05:39:46PM +0100, Hans de Goede wrote: > >>--- > >> server/Makefile.am | 2 + > >> server/agent-msg-filter.c | 85 > >> +++++++++++++++++++++++++++++++++++++++++++++ > >> server/agent-msg-filter.h | 45 ++++++++++++++++++++++++ > >> server/reds.c | 32 ++++++++++++++++- > >> 4 files changed, 162 insertions(+), 2 deletions(-) > >> create mode 100644 server/agent-msg-filter.c > >> create mode 100644 server/agent-msg-filter.h > >> > >>diff --git a/server/Makefile.am b/server/Makefile.am > >>index 7ab8c3d..37ff183 100644 > >>--- a/server/Makefile.am > >>+++ b/server/Makefile.am > >>@@ -95,6 +95,8 @@ SMARTCARD_SRCS = > >> endif > >> > >> libspice_server_la_SOURCES = \ > >>+ agent-msg-filter.c \ > >>+ agent-msg-filter.h \ > >> demarshallers.h \ > >> glz_encoder.c \ > >> glz_encoder_config.h \ > >>diff --git a/server/agent-msg-filter.c b/server/agent-msg-filter.c > >>new file mode 100644 > >>index 0000000..3867d11 > >>--- /dev/null > >>+++ b/server/agent-msg-filter.c > >>@@ -0,0 +1,85 @@ > >>+/* > >>+ Copyright (C) 2011 Red Hat, Inc. > >>+ > >>+ This library is free software; you can redistribute it and/or > >>+ modify it under the terms of the GNU Lesser General Public > >>+ License as published by the Free Software Foundation; either > >>+ version 2.1 of the License, or (at your option) any later version. > >>+ > >>+ This library is distributed in the hope that it will be useful, > >>+ but WITHOUT ANY WARRANTY; without even the implied warranty of > >>+ MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU > >>+ Lesser General Public License for more details. > >>+ > >>+ You should have received a copy of the GNU Lesser General Public > >>+ License along with this library; if not, > >>see<http://www.gnu.org/licenses/>. > >>+ > >>+ Red Hat Authors: > >>+ [email protected] > >>+*/ > >>+ > >>+#include<string.h> > >>+#include "red_common.h" > >>+#include "agent-msg-filter.h" > >>+ > >>+void agent_msg_filter_init(struct AgentMsgFilter *filter, int copy_paste) > >>+{ > >>+ memset(filter, 0, sizeof(*filter)); > > > >You are explicitly relying on AGENT_MSG_FILTER_OK == 0, would be nice > >to set filter->Result = AGENT_MSG_FILTER_OK explicitly. > > > > Er, no result will not be used until we've gone through the switch case in > agent_msg_filter_process_data at least once, at which point it has always > been explicitly set to one of the values from the enum. > > >>+ filter->copy_paste_enabled = copy_paste; > >>+} > >>+ > >>+int agent_msg_filter_process_data(struct AgentMsgFilter *filter, > >>+ uint8_t *data, uint32_t len) > >>+{ > >>+ struct VDAgentMessage msg_header; > >>+ > >>+ if (len> VD_AGENT_MAX_DATA_SIZE) { > >>+ red_printf("invalid agent message: too large"); > >>+ return AGENT_MSG_FILTER_PROTO_ERROR; > >>+ } > >>+ > >>+ /* Are we expecting more data from a previous message? */ > >>+ if (filter->msg_data_to_read) { > >>+data_to_read: > >>+ if (len> filter->msg_data_to_read) { > >>+ red_printf("invalid agent message: data exceeds size from > >>header"); > >>+ return AGENT_MSG_FILTER_PROTO_ERROR; > >>+ } > >>+ filter->msg_data_to_read -= len; > >>+ return filter->result; > >>+ } > >>+ > >>+ if (len< sizeof(msg_header)) { > > > >Is it impossible to get a partial write that is smaller then the size > >of the header? This also applies for a read with multiple messages > >ending with a partial header. > > > > Note that at the point agent_msg_filter_process_data gets called, the > (de)multiplexing from/into chunks has already been done. So we are being > fed data one chunk at a time. So what these checks check is that: > -The first chunk of a (possibly multi chunk spanning) message contains a > complete header > -The last chunk of a message does not push to total amount of message data > over the size specified in the messages header > > The linux vdagent has the same sanity checks. If either one of these 2 > fails, things are seriously wrong. > > Regards, > > Hans > _______________________________________________ > Spice-devel mailing list > [email protected] > http://lists.freedesktop.org/mailman/listinfo/spice-devel _______________________________________________ Spice-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/spice-devel
