>
> 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.
> + 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
if (count) {
memcpy(data, priv->read_buf, count);
}
Frediano
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel