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