diff mbox series

[04/16] vhost: prep vhost_dev_init users to handle failures

Message ID 1602104101-5592-5-git-send-email-michael.christie@oracle.com
State Superseded
Headers show
Series vhost: fix scsi cmd handling and IOPs | expand

Commit Message

Mike Christie Oct. 7, 2020, 8:54 p.m. UTC 
This is just a prep patch to get vhost_dev_init callers ready to handle
the next patch where the function can fail. In this patch vhost_dev_init
just returns 0, but I think it's easier to check for goto/error handling
errors separated from the next patch.

Signed-off-by: Mike Christie <michael.christie@oracle.com>
---
 drivers/vhost/net.c   | 11 +++++++----
 drivers/vhost/scsi.c  |  7 +++++--
 drivers/vhost/test.c  |  9 +++++++--
 drivers/vhost/vdpa.c  |  6 ++++--
 drivers/vhost/vhost.c | 14 ++++++++------
 drivers/vhost/vhost.h | 10 +++++-----
 drivers/vhost/vsock.c |  9 ++++++---
 7 files changed, 42 insertions(+), 24 deletions(-)

Comments

kernel test robot Oct. 8, 2020, 12:58 a.m. UTC | #1 
Hi Mike,

Thank you for the patch! Perhaps something to improve:

[auto build test WARNING on vhost/linux-next]
[also build test WARNING on next-20201007]
[cannot apply to v5.9-rc8]
[If your patch is applied to the wrong git tree, kindly drop us a note.
And when submitting patch, we suggest to use '--base' as documented in
https://git-scm.com/docs/git-format-patch]

url:    https://github.com/0day-ci/linux/commits/Mike-Christie/vhost-fix-scsi-cmd-handling-and-IOPs/20201008-045802
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: x86_64-randconfig-a016-20201008 (attached as .config)
compiler: clang version 12.0.0 (https://github.com/llvm/llvm-project 4d1d8ae7100ec3c7e1709addb7b3ec6f9ad0b44f)
reproduce (this is a W=1 build):
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # install x86_64 cross compiling tool for clang build
        # apt-get install binutils-x86-64-linux-gnu
        # https://github.com/0day-ci/linux/commit/7dc4d1082d406f391238a1897cb030f33c382bc3
        git remote add linux-review https://github.com/0day-ci/linux
        git fetch --no-tags linux-review Mike-Christie/vhost-fix-scsi-cmd-handling-and-IOPs/20201008-045802
        git checkout 7dc4d1082d406f391238a1897cb030f33c382bc3
        # save the attached .config to linux build tree
        COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross ARCH=x86_64 

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>

All warnings (new ones prefixed by >>):

>> drivers/vhost/vdpa.c:820:6: warning: variable 'r' is used uninitialized whenever 'if' condition is true [-Wsometimes-uninitialized]
           if (vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,
               ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/vhost/vdpa.c:844:9: note: uninitialized use occurs here
           return r;
                  ^
   drivers/vhost/vdpa.c:820:2: note: remove the 'if' if its condition is always false
           if (vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,
           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   drivers/vhost/vdpa.c:798:16: note: initialize the variable 'r' to silence this warning
           int nvqs, i, r, opened;
                         ^
                          = 0
   1 warning generated.

vim +820 drivers/vhost/vdpa.c

   792	
   793	static int vhost_vdpa_open(struct inode *inode, struct file *filep)
   794	{
   795		struct vhost_vdpa *v;
   796		struct vhost_dev *dev;
   797		struct vhost_virtqueue **vqs;
   798		int nvqs, i, r, opened;
   799	
   800		v = container_of(inode->i_cdev, struct vhost_vdpa, cdev);
   801	
   802		opened = atomic_cmpxchg(&v->opened, 0, 1);
   803		if (opened)
   804			return -EBUSY;
   805	
   806		nvqs = v->nvqs;
   807		vhost_vdpa_reset(v);
   808	
   809		vqs = kmalloc_array(nvqs, sizeof(*vqs), GFP_KERNEL);
   810		if (!vqs) {
   811			r = -ENOMEM;
   812			goto err;
   813		}
   814	
   815		dev = &v->vdev;
   816		for (i = 0; i < nvqs; i++) {
   817			vqs[i] = &v->vqs[i];
   818			vqs[i]->handle_kick = handle_vq_kick;
   819		}
 > 820		if (vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,
   821				   vhost_vdpa_process_iotlb_msg))
   822			goto err_dev_init;
   823	
   824		dev->iotlb = vhost_iotlb_alloc(0, 0);
   825		if (!dev->iotlb) {
   826			r = -ENOMEM;
   827			goto err_init_iotlb;
   828		}
   829	
   830		r = vhost_vdpa_alloc_domain(v);
   831		if (r)
   832			goto err_init_iotlb;
   833	
   834		filep->private_data = v;
   835	
   836		return 0;
   837	
   838	err_init_iotlb:
   839		vhost_dev_cleanup(&v->vdev);
   840	err_dev_init:
   841		kfree(vqs);
   842	err:
   843		atomic_dec(&v->opened);
   844		return r;
   845	}
   846	

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Dan Carpenter Oct. 9, 2020, 11:41 a.m. UTC | #2 
Hi Mike,

url:    https://github.com/0day-ci/linux/commits/Mike-Christie/vhost-fix-scsi-cmd-handling-and-IOPs/20201008-045802
base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next
config: x86_64-randconfig-m001-20201008 (attached as .config)
compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot <lkp@intel.com>
Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

New smatch warnings:
drivers/vhost/vdpa.c:844 vhost_vdpa_open() error: uninitialized symbol 'r'.

Old smatch warnings:
drivers/vhost/vdpa.c:436 vhost_vdpa_unlocked_ioctl() warn: maybe return -EFAULT instead of the bytes remaining?
drivers/vhost/vdpa.c:489 vhost_vdpa_unlocked_ioctl() warn: maybe return -EFAULT instead of the bytes remaining?

vim +/r +844 drivers/vhost/vdpa.c

4c8cf31885f69e8 Tiwei Bie     2020-03-26  793  static int vhost_vdpa_open(struct inode *inode, struct file *filep)
4c8cf31885f69e8 Tiwei Bie     2020-03-26  794  {
4c8cf31885f69e8 Tiwei Bie     2020-03-26  795  	struct vhost_vdpa *v;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  796  	struct vhost_dev *dev;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  797  	struct vhost_virtqueue **vqs;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  798  	int nvqs, i, r, opened;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  799  
4c8cf31885f69e8 Tiwei Bie     2020-03-26  800  	v = container_of(inode->i_cdev, struct vhost_vdpa, cdev);
4c8cf31885f69e8 Tiwei Bie     2020-03-26  801  
4c8cf31885f69e8 Tiwei Bie     2020-03-26  802  	opened = atomic_cmpxchg(&v->opened, 0, 1);
4c8cf31885f69e8 Tiwei Bie     2020-03-26  803  	if (opened)
4c8cf31885f69e8 Tiwei Bie     2020-03-26  804  		return -EBUSY;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  805  
4c8cf31885f69e8 Tiwei Bie     2020-03-26  806  	nvqs = v->nvqs;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  807  	vhost_vdpa_reset(v);
4c8cf31885f69e8 Tiwei Bie     2020-03-26  808  
4c8cf31885f69e8 Tiwei Bie     2020-03-26  809  	vqs = kmalloc_array(nvqs, sizeof(*vqs), GFP_KERNEL);
4c8cf31885f69e8 Tiwei Bie     2020-03-26  810  	if (!vqs) {
4c8cf31885f69e8 Tiwei Bie     2020-03-26  811  		r = -ENOMEM;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  812  		goto err;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  813  	}
4c8cf31885f69e8 Tiwei Bie     2020-03-26  814  
4c8cf31885f69e8 Tiwei Bie     2020-03-26  815  	dev = &v->vdev;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  816  	for (i = 0; i < nvqs; i++) {
4c8cf31885f69e8 Tiwei Bie     2020-03-26  817  		vqs[i] = &v->vqs[i];
4c8cf31885f69e8 Tiwei Bie     2020-03-26  818  		vqs[i]->handle_kick = handle_vq_kick;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  819  	}
7dc4d1082d406f3 Mike Christie 2020-10-07  820  	if (vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,
7dc4d1082d406f3 Mike Christie 2020-10-07  821  			   vhost_vdpa_process_iotlb_msg))
7dc4d1082d406f3 Mike Christie 2020-10-07  822  		goto err_dev_init;

"r" not set on this error path.

4c8cf31885f69e8 Tiwei Bie     2020-03-26  823  
4c8cf31885f69e8 Tiwei Bie     2020-03-26  824  	dev->iotlb = vhost_iotlb_alloc(0, 0);
4c8cf31885f69e8 Tiwei Bie     2020-03-26  825  	if (!dev->iotlb) {
4c8cf31885f69e8 Tiwei Bie     2020-03-26  826  		r = -ENOMEM;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  827  		goto err_init_iotlb;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  828  	}
4c8cf31885f69e8 Tiwei Bie     2020-03-26  829  
4c8cf31885f69e8 Tiwei Bie     2020-03-26  830  	r = vhost_vdpa_alloc_domain(v);
4c8cf31885f69e8 Tiwei Bie     2020-03-26  831  	if (r)
4c8cf31885f69e8 Tiwei Bie     2020-03-26  832  		goto err_init_iotlb;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  833  
4c8cf31885f69e8 Tiwei Bie     2020-03-26  834  	filep->private_data = v;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  835  
4c8cf31885f69e8 Tiwei Bie     2020-03-26  836  	return 0;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  837  
4c8cf31885f69e8 Tiwei Bie     2020-03-26  838  err_init_iotlb:
4c8cf31885f69e8 Tiwei Bie     2020-03-26  839  	vhost_dev_cleanup(&v->vdev);
7dc4d1082d406f3 Mike Christie 2020-10-07  840  err_dev_init:
37787e9f81e2e58 Mike Christie 2020-09-21  841  	kfree(vqs);
4c8cf31885f69e8 Tiwei Bie     2020-03-26  842  err:
4c8cf31885f69e8 Tiwei Bie     2020-03-26  843  	atomic_dec(&v->opened);
4c8cf31885f69e8 Tiwei Bie     2020-03-26 @844  	return r;
4c8cf31885f69e8 Tiwei Bie     2020-03-26  845  }

---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Michael S. Tsirkin Oct. 23, 2020, 3:56 p.m. UTC | #3 
On Fri, Oct 09, 2020 at 02:41:26PM +0300, Dan Carpenter wrote:
> Hi Mike,

> 

> url:    https://github.com/0day-ci/linux/commits/Mike-Christie/vhost-fix-scsi-cmd-handling-and-IOPs/20201008-045802

> base:   https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git linux-next

> config: x86_64-randconfig-m001-20201008 (attached as .config)

> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

> 

> If you fix the issue, kindly add following tag as appropriate

> Reported-by: kernel test robot <lkp@intel.com>

> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

> 

> New smatch warnings:

> drivers/vhost/vdpa.c:844 vhost_vdpa_open() error: uninitialized symbol 'r'.

> 

> Old smatch warnings:

> drivers/vhost/vdpa.c:436 vhost_vdpa_unlocked_ioctl() warn: maybe return -EFAULT instead of the bytes remaining?

> drivers/vhost/vdpa.c:489 vhost_vdpa_unlocked_ioctl() warn: maybe return -EFAULT instead of the bytes remaining?

> 

> vim +/r +844 drivers/vhost/vdpa.c

> 

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  793  static int vhost_vdpa_open(struct inode *inode, struct file *filep)

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  794  {

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  795  	struct vhost_vdpa *v;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  796  	struct vhost_dev *dev;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  797  	struct vhost_virtqueue **vqs;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  798  	int nvqs, i, r, opened;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  799  

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  800  	v = container_of(inode->i_cdev, struct vhost_vdpa, cdev);

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  801  

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  802  	opened = atomic_cmpxchg(&v->opened, 0, 1);

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  803  	if (opened)

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  804  		return -EBUSY;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  805  

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  806  	nvqs = v->nvqs;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  807  	vhost_vdpa_reset(v);

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  808  

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  809  	vqs = kmalloc_array(nvqs, sizeof(*vqs), GFP_KERNEL);

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  810  	if (!vqs) {

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  811  		r = -ENOMEM;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  812  		goto err;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  813  	}

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  814  

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  815  	dev = &v->vdev;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  816  	for (i = 0; i < nvqs; i++) {

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  817  		vqs[i] = &v->vqs[i];

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  818  		vqs[i]->handle_kick = handle_vq_kick;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  819  	}

> 7dc4d1082d406f3 Mike Christie 2020-10-07  820  	if (vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,

> 7dc4d1082d406f3 Mike Christie 2020-10-07  821  			   vhost_vdpa_process_iotlb_msg))

> 7dc4d1082d406f3 Mike Christie 2020-10-07  822  		goto err_dev_init;

> 

> "r" not set on this error path.

> 

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  823  

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  824  	dev->iotlb = vhost_iotlb_alloc(0, 0);

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  825  	if (!dev->iotlb) {

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  826  		r = -ENOMEM;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  827  		goto err_init_iotlb;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  828  	}

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  829  

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  830  	r = vhost_vdpa_alloc_domain(v);

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  831  	if (r)

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  832  		goto err_init_iotlb;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  833  

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  834  	filep->private_data = v;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  835  

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  836  	return 0;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  837  

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  838  err_init_iotlb:

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  839  	vhost_dev_cleanup(&v->vdev);

> 7dc4d1082d406f3 Mike Christie 2020-10-07  840  err_dev_init:

> 37787e9f81e2e58 Mike Christie 2020-09-21  841  	kfree(vqs);

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  842  err:

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  843  	atomic_dec(&v->opened);

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26 @844  	return r;

> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  845  }



Yes looks like it would use r uninitialized ...
Mike?

> ---

> 0-DAY CI Kernel Test Service, Intel Corporation

> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
Mike Christie Oct. 23, 2020, 4:21 p.m. UTC | #4 
On 10/23/20 10:56 AM, Michael S. Tsirkin wrote:
> On Fri, Oct 09, 2020 at 02:41:26PM +0300, Dan Carpenter wrote:

>> Hi Mike,

>>

>> url:    https://urldefense.com/v3/__https://github.com/0day-ci/linux/commits/Mike-Christie/vhost-fix-scsi-cmd-handling-and-IOPs/20201008-045802__;!!GqivPVa7Brio!PSUeg8MO8B2TLNpewKuGd0oWY8N3pkO7w9hbCh3xgWK3TgFsPr78zcIUZ8Orgxgaqydf$

>> base:   https://urldefense.com/v3/__https://git.kernel.org/pub/scm/linux/kernel/git/mst/vhost.git__;!!GqivPVa7Brio!PSUeg8MO8B2TLNpewKuGd0oWY8N3pkO7w9hbCh3xgWK3TgFsPr78zcIUZ8Org7WbKd27$  linux-next

>> config: x86_64-randconfig-m001-20201008 (attached as .config)

>> compiler: gcc-9 (Debian 9.3.0-15) 9.3.0

>>

>> If you fix the issue, kindly add following tag as appropriate

>> Reported-by: kernel test robot <lkp@intel.com>

>> Reported-by: Dan Carpenter <dan.carpenter@oracle.com>

>>

>> New smatch warnings:

>> drivers/vhost/vdpa.c:844 vhost_vdpa_open() error: uninitialized symbol 'r'.

>>

>> Old smatch warnings:

>> drivers/vhost/vdpa.c:436 vhost_vdpa_unlocked_ioctl() warn: maybe return -EFAULT instead of the bytes remaining?

>> drivers/vhost/vdpa.c:489 vhost_vdpa_unlocked_ioctl() warn: maybe return -EFAULT instead of the bytes remaining?

>>

>> vim +/r +844 drivers/vhost/vdpa.c

>>

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  793  static int vhost_vdpa_open(struct inode *inode, struct file *filep)

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  794  {

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  795  	struct vhost_vdpa *v;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  796  	struct vhost_dev *dev;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  797  	struct vhost_virtqueue **vqs;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  798  	int nvqs, i, r, opened;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  799

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  800  	v = container_of(inode->i_cdev, struct vhost_vdpa, cdev);

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  801

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  802  	opened = atomic_cmpxchg(&v->opened, 0, 1);

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  803  	if (opened)

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  804  		return -EBUSY;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  805

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  806  	nvqs = v->nvqs;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  807  	vhost_vdpa_reset(v);

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  808

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  809  	vqs = kmalloc_array(nvqs, sizeof(*vqs), GFP_KERNEL);

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  810  	if (!vqs) {

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  811  		r = -ENOMEM;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  812  		goto err;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  813  	}

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  814

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  815  	dev = &v->vdev;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  816  	for (i = 0; i < nvqs; i++) {

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  817  		vqs[i] = &v->vqs[i];

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  818  		vqs[i]->handle_kick = handle_vq_kick;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  819  	}

>> 7dc4d1082d406f3 Mike Christie 2020-10-07  820  	if (vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,

>> 7dc4d1082d406f3 Mike Christie 2020-10-07  821  			   vhost_vdpa_process_iotlb_msg))

>> 7dc4d1082d406f3 Mike Christie 2020-10-07  822  		goto err_dev_init;

>>

>> "r" not set on this error path.

>>

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  823

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  824  	dev->iotlb = vhost_iotlb_alloc(0, 0);

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  825  	if (!dev->iotlb) {

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  826  		r = -ENOMEM;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  827  		goto err_init_iotlb;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  828  	}

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  829

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  830  	r = vhost_vdpa_alloc_domain(v);

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  831  	if (r)

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  832  		goto err_init_iotlb;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  833

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  834  	filep->private_data = v;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  835

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  836  	return 0;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  837

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  838  err_init_iotlb:

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  839  	vhost_dev_cleanup(&v->vdev);

>> 7dc4d1082d406f3 Mike Christie 2020-10-07  840  err_dev_init:

>> 37787e9f81e2e58 Mike Christie 2020-09-21  841  	kfree(vqs);

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  842  err:

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  843  	atomic_dec(&v->opened);

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26 @844  	return r;

>> 4c8cf31885f69e8 Tiwei Bie     2020-03-26  845  }

> 

> 

> Yes looks like it would use r uninitialized ...

> Mike?

> 


Ah sorry, I had posted a v3 of this patchset:

https://patchwork.kernel.org/project/target-devel/list/?series=368487

and I fixed that but there was another cases of uninitialized variable 
that I missed. I fixed that up now, but have not posted an updated set.
diff mbox series

Patch

diff --git a/drivers/vhost/net.c b/drivers/vhost/net.c
index 831d824..fd30b53 100644
--- a/drivers/vhost/net.c
+++ b/drivers/vhost/net.c
@@ -1316,10 +1316,11 @@  static int vhost_net_open(struct inode *inode, struct file *f)
 		n->vqs[i].rx_ring = NULL;
 		vhost_net_buf_init(&n->vqs[i].rxq);
 	}
-	vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX,
-		       UIO_MAXIOV + VHOST_NET_BATCH,
-		       VHOST_NET_PKT_WEIGHT, VHOST_NET_WEIGHT, true,
-		       NULL);
+	if (vhost_dev_init(dev, vqs, VHOST_NET_VQ_MAX,
+			   UIO_MAXIOV + VHOST_NET_BATCH,
+			   VHOST_NET_PKT_WEIGHT, VHOST_NET_WEIGHT, true,
+			   NULL))
+		goto err_dev_init;
 
 	vhost_poll_init(n->poll + VHOST_NET_VQ_TX, handle_tx_net, EPOLLOUT, dev);
 	vhost_poll_init(n->poll + VHOST_NET_VQ_RX, handle_rx_net, EPOLLIN, dev);
@@ -1330,6 +1331,8 @@  static int vhost_net_open(struct inode *inode, struct file *f)
 
 	return 0;
 
+err_dev_init:
+	kfree(xdp);
 err_xdp:
 	kfree(queue);
 err_queue:
diff --git a/drivers/vhost/scsi.c b/drivers/vhost/scsi.c
index 86617bb..63ba363 100644
--- a/drivers/vhost/scsi.c
+++ b/drivers/vhost/scsi.c
@@ -1632,14 +1632,17 @@  static int vhost_scsi_open(struct inode *inode, struct file *f)
 		vqs[i] = &vs->vqs[i].vq;
 		vs->vqs[i].vq.handle_kick = vhost_scsi_handle_kick;
 	}
-	vhost_dev_init(&vs->dev, vqs, VHOST_SCSI_MAX_VQ, UIO_MAXIOV,
-		       VHOST_SCSI_WEIGHT, 0, true, NULL);
+	if (vhost_dev_init(&vs->dev, vqs, VHOST_SCSI_MAX_VQ, UIO_MAXIOV,
+			   VHOST_SCSI_WEIGHT, 0, true, NULL))
+		goto err_dev_init;
 
 	vhost_scsi_init_inflight(vs, NULL);
 
 	f->private_data = vs;
 	return 0;
 
+err_dev_init:
+	kfree(vqs);
 err_vqs:
 	kvfree(vs);
 err_vs:
diff --git a/drivers/vhost/test.c b/drivers/vhost/test.c
index a09dedc..c255ae5 100644
--- a/drivers/vhost/test.c
+++ b/drivers/vhost/test.c
@@ -119,12 +119,17 @@  static int vhost_test_open(struct inode *inode, struct file *f)
 	dev = &n->dev;
 	vqs[VHOST_TEST_VQ] = &n->vqs[VHOST_TEST_VQ];
 	n->vqs[VHOST_TEST_VQ].handle_kick = handle_vq_kick;
-	vhost_dev_init(dev, vqs, VHOST_TEST_VQ_MAX, UIO_MAXIOV,
-		       VHOST_TEST_PKT_WEIGHT, VHOST_TEST_WEIGHT, true, NULL);
+	if (vhost_dev_init(dev, vqs, VHOST_TEST_VQ_MAX, UIO_MAXIOV,
+			   VHOST_TEST_PKT_WEIGHT, VHOST_TEST_WEIGHT, true, NULL)
+		goto err_dev_init;
 
 	f->private_data = n;
 
 	return 0;
+
+err_dev_init:
+	kfree(vqs);
+	return -ENOMEM;
 }
 
 static void *vhost_test_stop_vq(struct vhost_test *n,
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index 62a9bb0..d413ceb 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -817,8 +817,9 @@  static int vhost_vdpa_open(struct inode *inode, struct file *filep)
 		vqs[i] = &v->vqs[i];
 		vqs[i]->handle_kick = handle_vq_kick;
 	}
-	vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,
-		       vhost_vdpa_process_iotlb_msg);
+	if (vhost_dev_init(dev, vqs, nvqs, 0, 0, 0, false,
+			   vhost_vdpa_process_iotlb_msg))
+		goto err_dev_init;
 
 	dev->iotlb = vhost_iotlb_alloc(0, 0);
 	if (!dev->iotlb) {
@@ -836,6 +837,7 @@  static int vhost_vdpa_open(struct inode *inode, struct file *filep)
 
 err_init_iotlb:
 	vhost_dev_cleanup(&v->vdev);
+err_dev_init:
 	kfree(vqs);
 err:
 	atomic_dec(&v->opened);
diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index fbb66f6..b05e690 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -459,12 +459,12 @@  static size_t vhost_get_desc_size(struct vhost_virtqueue *vq,
 	return sizeof(*vq->desc) * num;
 }
 
-void vhost_dev_init(struct vhost_dev *dev,
-		    struct vhost_virtqueue **vqs, int nvqs,
-		    int iov_limit, int weight, int byte_weight,
-		    bool use_worker,
-		    int (*msg_handler)(struct vhost_dev *dev,
-				       struct vhost_iotlb_msg *msg))
+int vhost_dev_init(struct vhost_dev *dev,
+		   struct vhost_virtqueue **vqs, int nvqs,
+		   int iov_limit, int weight, int byte_weight,
+		   bool use_worker,
+		   int (*msg_handler)(struct vhost_dev *dev,
+				      struct vhost_iotlb_msg *msg))
 {
 	struct vhost_virtqueue *vq;
 	int i;
@@ -501,6 +501,8 @@  void vhost_dev_init(struct vhost_dev *dev,
 			vhost_poll_init(&vq->poll, vq->handle_kick,
 					EPOLLIN, dev);
 	}
+
+	return 0;
 }
 EXPORT_SYMBOL_GPL(vhost_dev_init);
 
diff --git a/drivers/vhost/vhost.h b/drivers/vhost/vhost.h
index 11db183..a053318 100644
--- a/drivers/vhost/vhost.h
+++ b/drivers/vhost/vhost.h
@@ -167,11 +167,11 @@  struct vhost_dev {
 };
 
 bool vhost_exceeds_weight(struct vhost_virtqueue *vq, int pkts, int total_len);
-void vhost_dev_init(struct vhost_dev *, struct vhost_virtqueue **vqs,
-		    int nvqs, int iov_limit, int weight, int byte_weight,
-		    bool use_worker,
-		    int (*msg_handler)(struct vhost_dev *dev,
-				       struct vhost_iotlb_msg *msg));
+int vhost_dev_init(struct vhost_dev *dev, struct vhost_virtqueue **vqs,
+		   int nvqs, int iov_limit, int weight, int byte_weight,
+		   bool use_worker,
+		   int (*msg_handler)(struct vhost_dev *dev,
+				      struct vhost_iotlb_msg *msg));
 long vhost_dev_set_owner(struct vhost_dev *dev);
 bool vhost_dev_has_owner(struct vhost_dev *dev);
 long vhost_dev_check_owner(struct vhost_dev *);
diff --git a/drivers/vhost/vsock.c b/drivers/vhost/vsock.c
index f40205f..a1a35e1 100644
--- a/drivers/vhost/vsock.c
+++ b/drivers/vhost/vsock.c
@@ -630,9 +630,10 @@  static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
 	vsock->vqs[VSOCK_VQ_TX].handle_kick = vhost_vsock_handle_tx_kick;
 	vsock->vqs[VSOCK_VQ_RX].handle_kick = vhost_vsock_handle_rx_kick;
 
-	vhost_dev_init(&vsock->dev, vqs, ARRAY_SIZE(vsock->vqs),
-		       UIO_MAXIOV, VHOST_VSOCK_PKT_WEIGHT,
-		       VHOST_VSOCK_WEIGHT, true, NULL);
+	if (vhost_dev_init(&vsock->dev, vqs, ARRAY_SIZE(vsock->vqs),
+			   UIO_MAXIOV, VHOST_VSOCK_PKT_WEIGHT,
+			   VHOST_VSOCK_WEIGHT, true, NULL))
+		goto err_dev_init;
 
 	file->private_data = vsock;
 	spin_lock_init(&vsock->send_pkt_list_lock);
@@ -640,6 +641,8 @@  static int vhost_vsock_dev_open(struct inode *inode, struct file *file)
 	vhost_work_init(&vsock->send_pkt_work, vhost_transport_send_pkt_work);
 	return 0;
 
+err_dev_init:
+	kfree(vqs);
 out:
 	vhost_vsock_free(vsock);
 	return ret;