[Spice-devel] [spice-gtk v1] channel-usbredir: avoid calling memcpy() will NULL src

[Victor Toso]

Hi,

On Wed, Feb 28, 2018 at 12:41:42PM -0500, Frediano Ziglio wrote:
> > 
> > From: Victor Toso <me at victortoso.com>
> > 
> > Code built with address sanitizer has runtime error:
> >  > channel-usbredir.c:642:5: runtime error: null pointer passed
> >  > as argument 2, which is declared to never be null
> > 
> > Signed-off-by: Victor Toso <victortoso at redhat.com>
> > ---
> >  src/channel-usbredir.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> > 
> > diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c
> > index 1f791bc..7c48ecb 100644
> > --- a/src/channel-usbredir.c
> > +++ b/src/channel-usbredir.c
> > @@ -635,9 +635,9 @@ static int usbredir_read_callback(void *user_data,
> > uint8_t *data, int count)
> >      SpiceUsbredirChannel *channel = user_data;
> >      SpiceUsbredirChannelPrivate *priv = channel->priv;
> >  
> > -    if (priv->read_buf_size < count) {
> > -        count = priv->read_buf_size;
> > -    }
> > +    count = MIN(priv->read_buf_size, count);
> 
> Technically this part is just a style change but
> is clearly doing a minimum operation.

Yes, not related to the fix but the fix itself is to silence the
sanitizer.. so, I hope this is okay :)
> 
> > +    if (count == 0)
> > +        return 0;
> >  
> >      memcpy(data, priv->read_buf, count);
> >  
> 
> memcpy should not dereference any 0-byte area but I agree is better to
> silence the sanitizer and other tools.
> 
> Looking at the code there can be a side effects.
> If the usbredir send a 0-byte package you get read_buf_size == 0 and
> read_buf != NULL, processing this message lead to have read_buf != NULL
> now which can trigger a failure in usbredir_handle_msg (see code after
> the memcpy). Don't know if this is possible. Maybe is safer to do a

True

> 
>   if (count) {
>       memcpy(data, priv->read_buf, count);
>   }

Sure, will change to that!

        toso
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180228/bea28fde/attachment.sig>