Re: potential integer overflow in xenbus_file_write()

Ian Campbell
2012-10-15T10:25:57Z

On Thu, 2012-09-13 at 19:00 +0300, Dan Carpenter wrote:
> Hi,

Thanks Dan. I'm not sure anyone from Xen-land really monitors
virtualization@. Adding xen-devel and Konrad.

>
> I was reading some code and had a question in xenbus_file_write()
>
> drivers/xen/xenbus/xenbus_dev_frontend.c
> 461 if ((len + u->len) > sizeof(u->u.buffer)) {
> ^^^^^^^^^^^^
> Can this addition overflow?

len is a size_t and u->len is an unsigned int, so I expect so.

> Should the test be something like:
>
> if (len > sizeof(u->u.buffer) || len + u->len > sizeof(u->u.buffer)) {

I think that would do it.

Ian.

> 462 /* On error, dump existing buffer */
> 463 u->len = 0;
> 464 rc = -EINVAL;
> 465 goto out;
> 466 }
> 467
> 468 ret = copy_from_user(u->u.buffer + u->len, ubuf, len);
> 469
>
> regards,
> dan carpenter
> _______________________________________________
> Virtualization mailing list
> Virtualization@lists.linux-foundation.org
> https://lists.linuxfoundation.org/mailman/listinfo/virtualization
>