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

From: Dan Carpenter
Date: Wed Nov 30 2016 - 06:19:25 EST


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