Messages in this thread | | | Date | Wed, 30 Nov 2016 14:18:47 +0300 | From | Dan Carpenter <> | 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
| |