Re: [Xen-devel] potential integer overflow in xenbus_file_write()

2012-10-16 Thread Jan Beulich 
 On 15.10.12 at 12:25, Ian Campbell ian.campb...@citrix.com wrote:
 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.

Actually, it can remain a single range check:

if (len  sizeof(u-u.buffer) - u-len) {

because the rest of the code guarantees u-len to be less or
equal to sizeof(u-u.buffer) at all times.

Jan

___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

Re: potential integer overflow in xenbus_file_write()

2012-10-15 Thread Ian Campbell 
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
 


___
Virtualization mailing list
Virtualization@lists.linux-foundation.org
https://lists.linuxfoundation.org/mailman/listinfo/virtualization

potential integer overflow in xenbus_file_write()

2012-09-13 Thread Dan Carpenter 
Hi,

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?  Should the test be something like:

if (len  sizeof(u-u.buffer) || len + u-len  sizeof(u-u.buffer)) {

   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