[Spice-devel] [spice-server 04/10] Remove IncomingHandlerInterface::{alloc, release}_msg_buf

[Frediano Ziglio]

> 
> > 
> > On Wed, Feb 08, 2017 at 12:54:44PM -0600, Jonathon Jongsma wrote:
> > > On Tue, 2017-02-07 at 11:59 +0100, Christophe Fergeau wrote:
> > > > Similarly to the previous commits, this removes an indirection level,
> > > > IncomingHandlerInterface stores pointers to
> > > > alloc_recv_buf/release_recv_buf vfuncs, but these are always set to
> > > > RedChannel::alloc_recv_buf/RedChannel::release_recv_buf, which are
> > > > also
> > > > vfuncs which can be overridden if needed. This commit removes the
> > > > indirection and directly calls the relevant methods.
> > > > 
> > > > Not sure whether the corresponding vfuncs belong in
> > > > RedChannel or if they should be moved to RedChannelClient.
> > > 
> > > They do seem more appropriate to RedChannelClient, since the first
> > > argument is RedChannelClient*. If we do move them, it could be done in
> > > a subsequent commit, though. Maybe more of this stuff should be moved
> > > to the client?
> > 
> > A bit involved to move (eg the callbacks in CommonDisplayChannel
> > uses a buffer from CommonDisplayChannelPrivate, and there is no
> > CommonDisplayChannelClient, ...), so I'd leave that as improvements for
> > another time ;)
> > 
> > Christophe
> > 
> 
> Mumble mumble mumble... bug :-(
> 
> What happens with multiple clients sending data to these channels?
> The message buffer is shared between clients and so potentially
> the second client will overwrite the message coming from the first.
> Not likely to happen (only if message is read in chunk so large one
> which usually could be migration data, file transfer or usb).
> 
> Happily we don't support multiple clients (officially).
> Could this be used for exploits (like overwriting length)? I don't
> think so, messages from the same channel are handled in a single
> thread.
> 

Note that this issue is common to most of the channels.

Frediano