[Spice-devel] [PATCH spice-common] tests: Join test-overflow and test-marshallers
Frediano Ziglio
fziglio at redhat.com
Tue Jul 10 10:31:30 UTC 2018
>
> On Tue, Jul 10, 2018 at 04:51:35AM -0400, Frediano Ziglio wrote:
> > >
> > > On Tue, Jul 10, 2018 at 07:21:50AM +0100, Frediano Ziglio wrote:
> > > > test-overflow was doing a specific test on demarshalling code.
> > > > Joining the 2 tests also allows to remove the dependency from the main
> > > > protocol allowing to run the test independently from generation
> > > > setting.
> > >
> > >
> > >
> > > > This is useful with Meson allowing to not generate all code.
> > >
> > > Fwiw, I don't understand the rationale here.
> > >
> >
> > The "if spice_common_generate_code == 'all'" code in meson.build,
> > basically the test was only possible if the code compiled everything
>
> But why is this a problem? :) Because we only need to generate the code
> for tests/*.proto, and generating more is a waste of resources? Or is
> this a problem for other reasons?
>
> Christophe
>
Oh... I remember Eduardo wanting to not compile with spice_common_generate_code == 'all'
by default so to rnu the tests you would need to change the defaults.
By the way, I think this is a separate test as initially was in a security
series so detached from main development, I don't think we really need a
specific test to check a specific demarshalling issue.
> >
> > Frediano
> >
> > > Christophe
> > >
> > > >
> > > > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > > > ---
> > > > tests/Makefile.am | 17 ++-----
> > > > tests/meson.build | 6 +--
> > > > tests/test-marshallers.c | 81 +++++++++++++++++++++++++++++++--
> > > > tests/test-marshallers.h | 5 +++
> > > > tests/test-marshallers.proto | 5 +++
> > > > tests/test-overflow.c | 87 ------------------------------------
> > > > 6 files changed, 92 insertions(+), 109 deletions(-)
> > > > delete mode 100644 tests/test-overflow.c
> > > >
> > > > diff --git a/tests/Makefile.am b/tests/Makefile.am
> > > > index 1021954..926ac99 100644
> > > > --- a/tests/Makefile.am
> > > > +++ b/tests/Makefile.am
> > > > @@ -70,6 +70,7 @@ TEST_MARSHALLERS = \
> > > > generated_test_marshallers.c \
> > > > generated_test_marshallers.h \
> > > > generated_test_demarshallers.c \
> > > > + generated_test_enums.h \
> > > > $(NULL)
> > > >
> > > > BUILT_SOURCES = $(TEST_MARSHALLERS)
> > > > @@ -92,6 +93,8 @@ generated_test_marshallers.h:
> > > > $(srcdir)/test-marshallers.proto $(MARSHALLERS_DEP
> > > > $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
> > > > --generate-marshallers --server --include test-marshallers.h -H $< $@
> > > > >/dev/null
> > > > generated_test_demarshallers.c: $(srcdir)/test-marshallers.proto
> > > > $(MARSHALLERS_DEPS)
> > > > $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py
> > > > --generate-demarshallers --client --include test-marshallers.h $< $@
> > > > >/dev/null
> > > > +generated_test_enums.h: $(srcdir)/test-marshallers.proto
> > > > $(MARSHALLERS_DEPS)
> > > > + $(AM_V_GEN)$(PYTHON) $(top_srcdir)/spice_codegen.py -e $< $@
> > > > >/dev/null
> > > >
> > > > EXTRA_DIST = \
> > > > $(TEST_MARSHALLERS) \
> > > > @@ -99,18 +102,4 @@ EXTRA_DIST = \
> > > > test-marshallers.proto \
> > > > $(NULL)
> > > >
> > > > -TESTS += test_overflow
> > > > -test_overflow_SOURCES = test-overflow.c
> > > > -test_overflow_CFLAGS = \
> > > > - -I$(top_srcdir) \
> > > > - $(GLIB2_CFLAGS) \
> > > > - $(SPICE_COMMON_CFLAGS) \
> > > > - $(PROTOCOL_CFLAGS) \
> > > > - $(NULL)
> > > > -test_overflow_LDADD = \
> > > > - $(top_builddir)/common/libspice-common.la \
> > > > - $(top_builddir)/common/libspice-common-server.la \
> > > > - $(top_builddir)/common/libspice-common-client.la \
> > > > - $(NULL)
> > > > -
> > > > -include $(top_srcdir)/git.mk
> > > > diff --git a/tests/meson.build b/tests/meson.build
> > > > index 94c72c6..e53fd64 100644
> > > > --- a/tests/meson.build
> > > > +++ b/tests/meson.build
> > > > @@ -4,11 +4,6 @@
> > > > tests = ['test-logging', 'test-region']
> > > > tests_deps = [spice_common_dep]
> > > >
> > > > -if spice_common_generate_code == 'all'
> > > > - tests += ['test-overflow']
> > > > - tests_deps += [spice_common_client_dep, spice_common_server_dep]
> > > > -endif
> > > > -
> > > > foreach t : tests
> > > > name = t.underscorify()
> > > > exe = executable(name, '@0 at .c'.format(t),
> > > > @@ -28,6 +23,7 @@ targets = [
> > > > ['test_marshallers', test_proto, 'generated_test_marshallers.c',
> > > > ['--generate-marshallers', '--server', '--include',
> > > > 'test-marshallers.h', '@INPUT@', '@OUTPUT@']],
> > > > ['test_marshallers_h', test_proto, 'generated_test_marshallers.h',
> > > > ['--generate-marshallers', '--server', '--include',
> > > > 'test-marshallers.h', '-H', '@INPUT@', '@OUTPUT@']],
> > > > ['test_demarshallers', test_proto,
> > > > 'generated_test_demarshallers.c',
> > > > ['--generate-demarshallers', '--client', '--include',
> > > > 'test-marshallers.h', '@INPUT@', '@OUTPUT@']],
> > > > + ['test_enums_h', test_proto, 'generated_test_enums.h', ['-e',
> > > > '@INPUT@', '@OUTPUT@']],
> > > > ]
> > > >
> > > > foreach t : targets
> > > > diff --git a/tests/test-marshallers.c b/tests/test-marshallers.c
> > > > index 3bd98e8..ad45e36 100644
> > > > --- a/tests/test-marshallers.c
> > > > +++ b/tests/test-marshallers.c
> > > > @@ -1,13 +1,85 @@
> > > > +/*
> > > > + Copyright (C) 2015-2018 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/>.
> > > > +*/
> > > > +#ifdef HAVE_CONFIG_H
> > > > +#include <config.h>
> > > > +#endif
> > > > +
> > > > #include <glib.h>
> > > > #include <string.h>
> > > >
> > > > -#include "common/marshaller.h"
> > > > +#include <common/marshaller.h>
> > > > +#include <common/client_demarshallers.h>
> > > > +#include "generated_test_enums.h"
> > > > #include "generated_test_marshallers.h"
> > > >
> > > > #ifndef g_assert_true
> > > > #define g_assert_true g_assert
> > > > #endif
> > > >
> > > > +#define NUM_CHANNELS 3u
> > > > +
> > > > +void test_overflow(SpiceMarshaller *m)
> > > > +{
> > > > + SpiceMsgChannels *msg;
> > > > + uint8_t *data, *out;
> > > > + size_t len;
> > > > + int to_free = 0;
> > > > + spice_parse_channel_func_t func;
> > > > + unsigned int max_message_type, n;
> > > > + message_destructor_t free_output;
> > > > +
> > > > + msg = (SpiceMsgChannels *) malloc(sizeof(SpiceMsgChannels) +
> > > > + NUM_CHANNELS * sizeof(uint16_t));
> > > > + g_assert_nonnull(msg);
> > > > +
> > > > + // build a message and marshal it
> > > > + msg->num_of_channels = NUM_CHANNELS;
> > > > + for (n = 0; n < NUM_CHANNELS; ++n) {
> > > > + msg->channels[n] = n + 1;
> > > > + }
> > > > + spice_marshall_msg_main_channels_list(m, msg);
> > > > +
> > > > + // get linear data
> > > > + data = spice_marshaller_linearize(m, 0, &len, &to_free);
> > > > + g_assert_nonnull(data);
> > > > +
> > > > + printf("output len %lu\n", (unsigned long) len);
> > > > +
> > > > + // hack: setting the number of channels in the marshalled message
> > > > to a
> > > > + // value that will cause overflow while parsing the message to
> > > > make
> > > > sure
> > > > + // that the parser can handle this situation
> > > > + *((uint32_t *) data) = 0x80000002u;
> > > > +
> > > > + // extract the message
> > > > + func = spice_get_server_channel_parser(SPICE_CHANNEL_MAIN,
> > > > &max_message_type);
> > > > + g_assert_nonnull(func);
> > > > + out = func(data, data+len, SPICE_MSG_MAIN_CHANNELS_LIST, 0, &len,
> > > > &free_output);
> > > > + g_assert_null(out);
> > > > +
> > > > + // cleanup
> > > > + if (to_free) {
> > > > + free(data);
> > > > + }
> > > > + if (out) {
> > > > + free_output(out);
> > > > + }
> > > > + free(msg);
> > > > +}
> > > > +
> > > > static uint8_t expected_data[] = { 123, /* dummy byte */
> > > > 0x02, 0x00, 0x00, 0x00, /*
> > > > data_size */
> > > > 0x09, 0x00, 0x00, 0x00, /* data
> > > > offset
> > > > */
> > > > @@ -47,8 +119,9 @@ int main(int argc G_GNUC_UNUSED, char **argv
> > > > G_GNUC_UNUSED)
> > > > g_free(msg);
> > > >
> > > > // test demarshaller
> > > > - msg = (SpiceMsgMainShortDataSubMarshall *) spice_parse_msg(data,
> > > > data
> > > > + len, 1, 1, 0,
> > > > -
> > > > &msg_len,
> > > > &free_message);
> > > > + msg = (SpiceMsgMainShortDataSubMarshall *)
> > > > + spice_parse_msg(data, data + len, SPICE_CHANNEL_MAIN,
> > > > SPICE_MSG_MAIN_SHORTDATASUBMARSHALL,
> > > > + 0, &msg_len, &free_message);
> > > >
> > > > g_assert_nonnull(msg);
> > > > g_assert_cmpint(msg->dummy_byte, ==, 123);
> > > > @@ -75,6 +148,8 @@ int main(int argc G_GNUC_UNUSED, char **argv
> > > > G_GNUC_UNUSED)
> > > > free(data);
> > > > }
> > > >
> > > > + test_overflow(marshaller);
> > > > +
> > > > spice_marshaller_destroy(marshaller);
> > > >
> > > > return 0;
> > > > diff --git a/tests/test-marshallers.h b/tests/test-marshallers.h
> > > > index 46263d7..99877c0 100644
> > > > --- a/tests/test-marshallers.h
> > > > +++ b/tests/test-marshallers.h
> > > > @@ -16,5 +16,10 @@ typedef struct {
> > > > uint16_t n;
> > > > } SpiceMsgMainZeroes;
> > > >
> > > > +typedef struct SpiceMsgChannels {
> > > > + uint32_t num_of_channels;
> > > > + uint16_t channels[0];
> > > > +} SpiceMsgChannels;
> > > > +
> > > > #endif /* _H_TEST_MARSHALLERS */
> > > >
> > > > diff --git a/tests/test-marshallers.proto
> > > > b/tests/test-marshallers.proto
> > > > index 08d3e01..c75134e 100644
> > > > --- a/tests/test-marshallers.proto
> > > > +++ b/tests/test-marshallers.proto
> > > > @@ -14,6 +14,11 @@ channel TestChannel {
> > > > uint16 n;
> > > > uint32 res2 @zero;
> > > > } Zeroes;
> > > > +
> > > > + message {
> > > > + uint32 num_of_channels;
> > > > + uint16 channels[num_of_channels] @end;
> > > > + } @ctype(SpiceMsgChannels) channels_list;
> > > > };
> > > >
> > > > protocol Spice {
> > > > diff --git a/tests/test-overflow.c b/tests/test-overflow.c
> > > > deleted file mode 100644
> > > > index c972a25..0000000
> > > > --- a/tests/test-overflow.c
> > > > +++ /dev/null
> > > > @@ -1,87 +0,0 @@
> > > > -/*
> > > > - Copyright (C) 2015 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/>.
> > > > -*/
> > > > -#ifdef HAVE_CONFIG_H
> > > > -#include <config.h>
> > > > -#endif
> > > > -
> > > > -#include <stdio.h>
> > > > -#include <stdlib.h>
> > > > -#include <string.h>
> > > > -#include <assert.h>
> > > > -
> > > > -#include <spice/enums.h>
> > > > -#include <common/marshaller.h>
> > > > -#include <common/generated_server_marshallers.h>
> > > > -#include <common/client_demarshallers.h>
> > > > -
> > > > -#define NUM_CHANNELS 3u
> > > > -
> > > > -int main(void)
> > > > -{
> > > > - SpiceMarshaller *m;
> > > > - SpiceMsgChannels *msg;
> > > > - uint8_t *data, *out;
> > > > - size_t len;
> > > > - int to_free = 0;
> > > > - spice_parse_channel_func_t func;
> > > > - unsigned int max_message_type, n;
> > > > - message_destructor_t free_output;
> > > > -
> > > > - m = spice_marshaller_new();
> > > > - assert(m);
> > > > -
> > > > - msg = (SpiceMsgChannels *) malloc(sizeof(SpiceMsgChannels) +
> > > > - NUM_CHANNELS * sizeof(SpiceChannelId));
> > > > - assert(msg);
> > > > -
> > > > - // build a message and marshal it
> > > > - msg->num_of_channels = NUM_CHANNELS;
> > > > - for (n = 0; n < NUM_CHANNELS; ++n) {
> > > > - msg->channels[n] = (SpiceChannelId) { n + 1, n * 7 };
> > > > - }
> > > > - spice_marshall_msg_main_channels_list(m, msg);
> > > > -
> > > > - // get linear data
> > > > - data = spice_marshaller_linearize(m, 0, &len, &to_free);
> > > > - assert(data);
> > > > -
> > > > - printf("output len %lu\n", (unsigned long) len);
> > > > -
> > > > - // hack: setting the number of channels in the marshalled message
> > > > to a
> > > > - // value that will cause overflow while parsing the message to
> > > > make
> > > > sure
> > > > - // that the parser can handle this situation
> > > > - *((uint32_t *) data) = 0x80000002u;
> > > > -
> > > > - // extract the message
> > > > - func = spice_get_server_channel_parser(SPICE_CHANNEL_MAIN,
> > > > &max_message_type);
> > > > - assert(func);
> > > > - out = func(data, data+len, SPICE_MSG_MAIN_CHANNELS_LIST, 0, &len,
> > > > &free_output);
> > > > - assert(out == NULL);
> > > > -
> > > > - // cleanup
> > > > - if (to_free) {
> > > > - free(data);
> > > > - }
> > > > - if (out) {
> > > > - free_output(out);
> > > > - }
> > > > - free(msg);
> > > > - spice_marshaller_destroy(m);
> > > > -
> > > > - return 0;
> > > > -}
> > > > -
More information about the Spice-devel
mailing list