On 01/26/2017 02:50 PM, Dan Carpenter wrote:
> Hello Jike Song,
>
> The patch 659643f7d814: "drm/i915/gvt/kvmgt: add vfio/mdev support to
> KVMGT" from Dec 8, 2016, leads to the following static checker
> warning:
>
> drivers/gpu/drm/i915/gvt/kvmgt.c:969 intel_vgpu_ioctl()
> warn: calling kfree() when 'caps.buf' is always NULL.
>
> drivers/gpu/drm/i915/gvt/kvmgt.c
> 909 } else if (cmd == VFIO_DEVICE_GET_REGION_INFO) {
> 910 struct vfio_region_info info;
> 911 struct vfio_info_cap caps = { .buf = NULL, .size = 0 };
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> Set here.
>
> 912 int i, ret;
> 913 struct vfio_region_info_cap_sparse_mmap *sparse = NULL;
> 914 size_t size;
> 915 int nr_areas = 1;
> 916 int cap_type_id;
> 917
> 918 minsz = offsetofend(struct vfio_region_info, offset);
> 919
> 920 if (copy_from_user(&info, (void __user *)arg, minsz))
> 921 return -EFAULT;
> 922
> 923 if (info.argsz < minsz)
> 924 return -EINVAL;
> 925
> 926 switch (info.index) {
> 927 case VFIO_PCI_CONFIG_REGION_INDEX:
> 928 info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index);
> 929 info.size = INTEL_GVT_MAX_CFG_SPACE_SZ;
> 930 info.flags = VFIO_REGION_INFO_FLAG_READ |
> 931 VFIO_REGION_INFO_FLAG_WRITE;
> 932 break;
> 933 case VFIO_PCI_BAR0_REGION_INDEX:
> 934 info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index);
> 935 info.size = vgpu->cfg_space.bar[info.index].size;
> 936 if (!info.size) {
> 937 info.flags = 0;
> 938 break;
> 939 }
> 940
> 941 info.flags = VFIO_REGION_INFO_FLAG_READ |
> 942 VFIO_REGION_INFO_FLAG_WRITE;
> 943 break;
> 944 case VFIO_PCI_BAR1_REGION_INDEX:
> 945 info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index);
> 946 info.size = 0;
> 947 info.flags = 0;
> 948 break;
> 949 case VFIO_PCI_BAR2_REGION_INDEX:
> 950 info.offset = VFIO_PCI_INDEX_TO_OFFSET(info.index);
> 951 info.flags = VFIO_REGION_INFO_FLAG_CAPS |
> 952 VFIO_REGION_INFO_FLAG_MMAP |
> 953 VFIO_REGION_INFO_FLAG_READ |
> 954 VFIO_REGION_INFO_FLAG_WRITE;
> 955 info.size = gvt_aperture_sz(vgpu->gvt);
> 956
> 957 size = sizeof(*sparse) +
> 958 (nr_areas * sizeof(*sparse->areas));
> 959 sparse = kzalloc(size, GFP_KERNEL);
> 960 if (!sparse)
> 961 return -ENOMEM;
> 962
> 963 sparse->nr_areas = nr_areas;
> 964 cap_type_id = VFIO_REGION_INFO_CAP_SPARSE_MMAP;
> 965 sparse->areas[0].offset =
> 966 PAGE_ALIGN(vgpu_aperture_offset(vgpu));
> 967 sparse->areas[0].size = vgpu_aperture_sz(vgpu);
> 968 if (!caps.buf) {
> ^^^^^^^^
> It's always NULL.
>
> 969 kfree(caps.buf);
>
> Freeing NULL is pointless.
>
> 970 caps.buf = NULL;
> 971 caps.size = 0;
>
> These were already zeroed out at the start of the function. What was
> intended here? Probably you could just delete these lines.
>
Hi Dan,
Yes you are right, these are useless, probably a mistake in the development.
Curious what static checker do you use? I actually checked it with sparse but
this was not reported.
I'll submit a patch to remove that, thanks!
--
Thanks,
Jike
> 972 }
> 973 break;
> 974
> 975 case VFIO_PCI_BAR3_REGION_INDEX ... VFIO_PCI_BAR5_REGION_INDEX:
>
> regards,
> dan carpenter
>
_______________________________________________
Intel-gfx mailing list
Intel-gfx@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
