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