linux-kernel - [bug report] misc: mic: Enable VOP host side functionality

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20161130111847.GA27389@mwanda>
Date:   Wed, 30 Nov 2016 14:18:47 +0300
From:   Dan Carpenter <dan.carpenter@...cle.com>
To:     sudeep.dutt@...el.com
Cc:     Ashutosh Dixit <ashutosh.dixit@...el.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        linux-kernel@...r.kernel.org
Subject: [bug report] misc: mic: Enable VOP host side functionality

Hello Sudeep Dutt,

The patch 61e9c905df78: "misc: mic: Enable VOP host side
functionality" from Feb 8, 2016, leads to the following static
checker warning:

	drivers/misc/mic/vop/vop_vringh.c:961 vop_ioctl()
	warn: 'vdev' was already freed.

drivers/misc/mic/vop/vop_vringh.c
   271  static int vop_virtio_add_device(struct vop_vdev *vdev,
   272                                   struct mic_device_desc *argp)
   273  {
   274          struct vop_info *vi = vdev->vi;
   275          struct vop_device *vpdev = vi->vpdev;
   276          struct mic_device_desc *dd = NULL;
   277          struct mic_vqconfig *vqconfig;
   278          int vr_size, i, j, ret;
   279          u8 type = 0;
   280          s8 db = -1;
   281          char irqname[16];
   282          struct mic_bootparam *bootparam;
   283          u16 num;
   284          dma_addr_t vr_addr;
   285  
   286          bootparam = vpdev->hw_ops->get_dp(vpdev);
   287          init_waitqueue_head(&vdev->waitq);
   288          INIT_LIST_HEAD(&vdev->list);
   289          vdev->vpdev = vpdev;
   290  
   291          ret = vop_copy_dp_entry(vdev, argp, &type, &dd);
   292          if (ret) {
   293                  dev_err(vop_dev(vdev), "%s %d err %d\n",
   294                          __func__, __LINE__, ret);
   295                  kfree(vdev);

This is the only error path where we free "vdev".  I think it's a
mistake and we should just remove the kfree().

   296                  return ret;
   297          }
   298  

[ snip ]

   953                  mutex_lock(&vdev->vdev_mutex);
   954                  mutex_lock(&vi->vop_mutex);
   955                  ret = vop_virtio_add_device(vdev, dd_config);
   956                  if (ret)
   957                          goto unlock_ret;
   958                  list_add_tail(&vdev->list, &vi->vdev_list);
   959  unlock_ret:
   960                  mutex_unlock(&vi->vop_mutex);
   961                  mutex_unlock(&vdev->vdev_mutex);

The only caller still wants to use vdev, so it's a use after free if we
ever hit that return.

   962  free_ret:
   963                  kfree(dd_config);
   964                  return ret;
   965          }

regards,
dan carpenter