Hi, On Wed, Feb 28, 2018 at 12:41:42PM -0500, Frediano Ziglio wrote: > > > > From: Victor Toso <[email protected]> > > > > 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 <[email protected]> > > --- > > 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
signature.asc
Description: PGP signature
_______________________________________________ Spice-devel mailing list [email protected] https://lists.freedesktop.org/mailman/listinfo/spice-devel
