[Spice-devel] [PATCH spice-protocol] stream-device: Specify how padding shoud be inside new structures

[Frediano Ziglio]

> > On 23 Feb 2018, at 11:31, Christophe Fergeau <cfergeau at redhat.com> wrote:
> > 
> > On Fri, Feb 23, 2018 at 10:11:46AM +0000, Frediano Ziglio wrote:
> >> Depending on how structures are initialised in the code is
> >> possible that implicit padding bytes are not initialised
> >> causing possible information leaks as the entire structure
> >> with all padding is sent through device/network.
> >> 
> >> Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> >> ---
> >> spice/stream-device.h | 2 ++
> >> 1 file changed, 2 insertions(+)
> >> 
> >> diff --git a/spice/stream-device.h b/spice/stream-device.h
> >> index 2e7c50e..b2f83b5 100644
> >> --- a/spice/stream-device.h
> >> +++ b/spice/stream-device.h
> >> @@ -48,6 +48,8 @@
> >>  * containing integers up to 64 bit.
> >>  * All numbers are in little endian format.
> >>  *
> >> + * For security reasons structures should not contain implicit paddings.
> >> + *
> > 
> > Isn't padding inserted by the compiler going to be platform-dependent
> > anyway?
> 
> That is my concern too.
> 

Yes, every time you send data to a stream you are doing platform-dependent
implications (unless the transport send all bits and you have same
ABIs/encodings at both ends).

> > I would say that all structures used in the protocol should be packed.
> 

Yes, actually the "naturally aligned" requires it. Is a way to avoid misalignments.
The transportation in this case is supposed to be fast the space occupied
by padding is not an issue.
Considering that when TCP was designed machines had few MBs and still they
cared about alignment I don't see why should ignore in a fast transport.

One could complain about the endian and ask for a more guest friendly as
having the 2 ends with different endian here could mean full emulation where
doing endian conversion in the guest could be expensive, on the other hand
a protocol supporting both endianness could have some performance
impact and we mostly support little endian machines.

> I would also specify that what is sent is little-endian. While on x86, there
> is only one endianness, some platforms, e.g. Itanium, are bi-endian, so it
> is theoretically possible for the host to be big and the guest to be little
> for example. OK, I know, Itanium… :-)
> 

There's a "All numbers are in little endian format.".

I didn't specify the byte has 8 bit and the transport can carry full octets
but I think this is too paranoid.

> > 
> > Christophe
> > 
> >>  * The protocol can be defined by these states:
> >>  * - Initial. Device just opened. Guest should wait
> >>  *   for a message from the host;

Frediano