Check that combination of fields for an array does not
lead to unsafe code.
check_valid method came from generate_c_declaration with
some more check and it's use in demarshaller to validate
the array if the structure is not generated.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Victor Toso <victortoso@redhat.com>
This check make sure that output fields for member with @end (arrays)
are declared as empty arrays in output C structure.
This avoids output fields to be declared as pointer or other
invalid types.
The check is a compile time check so no code in object file
is generated.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
Just style, they do the same thing, but is more coherent
with the rest of the code.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Uri Lublin <uril@redhat.com>
The option is used to add a suffix to public functions, not a
prefix.
Currently the option is not used (it was used to generate protocol
1 code).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
@ptr_array is supposed to change the destination to an array
of pointer to items. This for a raw buffer does not make sense
but check if user specifies this combination.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Although on the platform we support size_t and uintptr_t are
the same, on some platform the size_t can (in theory) be smaller
than the necessary integer to store a pointer.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Code generated for demarshallers define and declare some types and
functions.
However these types and functions are also declared separately in other
headers (currently spice-common/client_demarshallers.h and
spice/server/demarshallers.h) resulting in potential ABI mismatch if the
different declarations do not match.
Using a common header shared between generated code and code using
these functions prevent potentially multiple different declarations.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
The idea in version 1 of the protocol was to extend it using the minor
version. However this was replaced by the usage of capabilities and the
minor attribute (which was not much used in version 1) was abandoned in
version 2.
This patch create a big difference in the code generated but only because
the minor version was passed between all possible functions as argument.
Note that exported functions retain the minor argument for compatibility
reasons.
The demarshaller code export directly spice_get_client_channel_parser or
spice_get_server_channel_parser functions which returns internal module
functions which parse message of specific channels.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This attribute was used only in SPICE version 1.
The intention was use fixed size for switch type in the protocol.
However this does not bring any improvement, just increase network
bytes used.
Generated code does not change.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
This attribute was used only in SPICE version 1.
Its usage was confusing, and was replaced by the simple usage of
array size.
Generated code does not change.
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Even though commit df4ec5c318 commented
out most of smartcard code which triggered this error, it still might
happen if a new message is added with an array member.
The reason is a missing declaration of mem_size, which is fixed simply
by checking if the attribute 'nocopy' is present.
The error log follows:
generated_server_demarshallers.c: In function ‘parse_msgc_smartcard_reader_add’:
generated_server_demarshallers.c:1985:30: error: ‘mem_size’ undeclared (first use in this function); did you mean ‘nw_size’?
data = (uint8_t *)malloc(mem_size);
^~~~~~~~
nw_size
This patch also updates test-marshallers so that this bug is triggered.
The diff between generated demarshallers with the patch applied follows:
--- tests/generated_test_demarshallers.c.old 2018-05-17 14:35:29.234056487 -0300
+++ tests/generated_test_demarshallers.c 2018-05-17 14:35:40.554031295 -0300
@@ -286,6 +286,7 @@ static uint8_t * parse_msg_main_ArrayMes
uint8_t *start = message_start;
uint8_t *data = NULL;
uint64_t nw_size;
+ uint64_t mem_size;
uint8_t *in, *end;
uint64_t name__nw_size;
uint64_t name__nelements;
@@ -298,6 +299,7 @@ static uint8_t * parse_msg_main_ArrayMes
}
nw_size = 0 + name__nw_size;
+ mem_size = sizeof(SpiceMsgMainArrayMessage);
/* Check if message fits in reported side */
if (nw_size > (uintptr_t) (message_end - start)) {
Signed-off-by: Eduardo Lima (Etrunko) <etrunko@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
Make code safe using both 32 and 64 bit machine.
Consider that this code can be compiled for machines with 32 bit.
There are some arrays length which are 32 bit.
If size_t this can cause easily an overflow. For instance message_len
sending SPICE_MSG_NOTIFY messages are 32 bit and code add a small
constant (currently 24) before doing the test for size. Now passing
(uint32_t) -20 as message_len would lead to a size of 4 after the
addition. This overflow does not happen on 64 bit machine as the length
is converted to size_t.
There are also some array length where some item are bigger than 1 byte.
For instance SPICE_MAIN_CHANNELS_LIST message have a number of channels
and each channel is composed by 2 bytes. Now the code generated try to do
length * 2 where length is still a 32 bit so if we put a value like
0x80000002u we get 4 as length. This will cause an overflow as code will
allocate very few bytes but try to fill with a huge number of elements.
This overflow happen in both 32 and 64 bit machine.
To avoid all these possible overflows this patch use only 64 bit for
nelements (number of elements), nw_size (network size) and mem_size
(memory size needed) checking the sizes to avoid other overflows
(like pointers conversions under 32 bit machines).
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe de Dinechin <dinechin@redhat.com>
Instead of assuming that the system can safely do unaligned access
to memory use packed structures to allow the compiler generate
best code possible.
A packed structure tells the compiler to not leave padding inside it
and that the structure can be unaligned so any field can be unaligned
having to generate proper access code based on architecture.
For instance ARM7 can use unaligned access but not for 64 bit
numbers (currently these accesses are emulated by Linux kernel
with obvious performance consequences).
This changes the current methods from:
#ifdef WORDS_BIGENDIAN
#define read_uint32(ptr) ((uint32_t)SPICE_BYTESWAP32(*((uint32_t *)(ptr))))
#define write_uint32(ptr, val) *(uint32_t *)(ptr) = SPICE_BYTESWAP32((uint32_t)val)
#else
#define read_uint32(ptr) (*((uint32_t *)(ptr)))
#define write_uint32(ptr, val) (*((uint32_t *)(ptr))) = val
#endif
to:
#include <spice/start-packed.h>
typedef struct SPICE_ATTR_PACKED {
uint32_t v;
} uint32_unaligned_t;
#include <spice/end-packed.h>
#ifdef WORDS_BIGENDIAN
#define read_uint32(ptr) ((uint32_t)SPICE_BYTESWAP32(((uint32_unaligned_t *)(ptr))->v))
#define write_uint32(ptr, val) ((uint32_unaligned_t *)(ptr))->v = SPICE_BYTESWAP32((uint32_t)val)
#else
#define read_uint32(ptr) (((uint32_unaligned_t *)(ptr))->v)
#define write_uint32(ptr, val) (((uint32_unaligned_t *)(ptr))->v) = val
#endif
Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
Acked-by: Christophe Fergeau <cfergeau@redhat.com>
Add a new type, "unix_fd", used to describe file descriptor sharing via
socket ancillary data (these messages are local only).
The marshaller/demarshaller can't serialize this in memory (consume_fd
implementation is empty), so it is the responsability of the marshaller
user to handle sending and receiving the handles, which are appended at
the end of the message with an extra stream byte (because some Unix
requires sending at least a byte with ancillary data).
Even if there is no fd to send (or if the fd is invalid etc), the
receiver side expects an extra byte anyway.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Acked-by: Frediano Ziglio <fziglio@redhat.com>
(cherry-picked from spice-protocol commit 267391c8fd7c90c067b3e4845ff0227a2580e2e2)
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
This reverts commit 7665dcf1bb.
Also revert the related build-sys changes to fix the build.
codegen generated code depends on spice-common code (marshaller,
messages etc), it makes more sense to keep the generator along
this. Otherwise a newer protocol release will fail to build older
projects.
*.proto files are required as well, since it generates code that parent
modules depend on unconditionnaly.
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
This is a new version of my previous patch that does not include six.py.
It's still kind of big, but at least it's all spice-common changes now.
There are also a few other fixes that Christophe brought to my attention.
Note that six now needs to be installed on the system (python-six on
Fedora and Debian, six on PyPI).
This *should* be enough to make spice_codegen.py work on both Python 2
and Python 3. The major changes are as follows:
* cStringIO.StringIO -> io.StringIO
* str vs. unicode updates (io.StringIO doesn't like str)
* integer division
* foo.has_key(bar) -> bar in foo
* import internal_thing -> from . import internal_thing
* removed from __future__ import with_statement
(might break Python 2.5?)
* changed some lambdas to list comprehensions (done by 2to3)
* cast some_dict.keys() to list where needed (e.g. for sorting)
* use normal type names with isinstance instead of types.WhateverType
Signed-off-by: Alexander Wauck <awauck@codeweavers.com>
Although the most part of the parameters marked as unused are actually
being used for a few functions, a bunch of warnings can be seen when
the code is compiled with "-Wall -Wextra". As adding the unused attribute
means that the variable/parameter is meant to be *possibly* unused, we're
safe adding it in the generated code, even for used variables/parameters.
Since the (de)marshallers are now generated in $builddir and not in
$srcdir, when these generated files include a file located in
$srcdir/common, the compiler will find them thanks to a -I directive, so it
makes more sense to use <> rather than "" when including them.
Now that they are created in $builddir, their includes will need to refer
to files in $srcdir, which can be different. It's cleaner to add
-I $(top_srcdir)/spice-common/ to modules using spice-common rather than
having -I $(top_srcdir)/spice-common/common which would could create header
collisions.
Before this patch, if a channel is defined conditionally in spice.proto
(because it depends on external headers like the smartcard channel),
spice_codegen would write an entry to the channels array in
spice_get_*_channel_parser which would only take up a place in the array
if the ifdef condition is true, thus moving up all other intializers
one place when it is not true. This was causing issues (crashes) when building
spice-gtk with the combination of usbredir support enabled and smartcard
support disabled.
This patch fixes this by adding #else { NULL, 0 }, to the generated code.
Thanks to coolper chen <lixin.chen@saicocch.com> for reporting this!
Signed-off-by: Hans de Goede <hdegoede@redhat.com>
A message with a fixed-size array, such as uint8 uuid[16] will
generate an invalid code, missing the __nelements variable. Make sure
that variable is defined.
python_modules/demarshal.py and marshal.py fixes for gcc 4.6.0
warning about set but unused variables. The fixes disable creating
of variables mem_size when they are not used (demarshall) and
declaring a src variable when the message doesn't use it (marshal).
You need to touch *.proto after applying this (should add a Makefile
dependency).
Patch adds a "from __future__" import that doesn't affect newer python's but
allows python 2.5.4 to run the code (tested under scratchbox, n900 build environment)
It turns out that using base + sizeof(struct) is not a good way to
access an array at the end of a struct. For SpicePalette sizeof is 16, but
offset of ents is 12. Using this calculation in the demarshaller breaks
things badly, so now we use the actual array member.
Remove all uses of @end in the marshaller, instead just using
the C struct array-at-end-of-struct. To make this work we also remove
all use of @end for switches (making them C unions).
We drop the zero member of the notify message so that we can avoid this
use of @end for a primitive in the marshaller (plus its useless to send
over the wire).
We change the offsets and stuff in the migration messages to real pointers.
Even for is_extra_size() we should calculate the mem_size for
arrays, its just that the parent type (in this case switch) should
request mem_size if the type is_extra_size.
