* [bug report] drm/i915/gvt/kvmgt: add vfio/mdev support to KVMGT
@ 2017-01-26 6:50 Dan Carpenter
2017-01-26 7:21 ` Jike Song
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2017-01-26 6:50 UTC (permalink / raw)
To: jike.song; +Cc: intel-gfx, igvt-g-dev
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.
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] drm/i915/gvt/kvmgt: add vfio/mdev support to KVMGT
2017-01-26 6:50 [bug report] drm/i915/gvt/kvmgt: add vfio/mdev support to KVMGT Dan Carpenter
@ 2017-01-26 7:21 ` Jike Song
2017-01-26 7:59 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Jike Song @ 2017-01-26 7:21 UTC (permalink / raw)
To: Dan Carpenter; +Cc: intel-gfx, igvt-g-dev
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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [bug report] drm/i915/gvt/kvmgt: add vfio/mdev support to KVMGT
2017-01-26 7:21 ` Jike Song
@ 2017-01-26 7:59 ` Dan Carpenter
0 siblings, 0 replies; 3+ messages in thread
From: Dan Carpenter @ 2017-01-26 7:59 UTC (permalink / raw)
To: Jike Song; +Cc: intel-gfx, igvt-g-dev
On Thu, Jan 26, 2017 at 03:21:06PM +0800, Jike Song wrote:
> Curious what static checker do you use? I actually checked it with sparse but
> this was not reported.
It's some unreleased Smatch stuff. It's not really useful except that
it's a good stress tester for Smatch to see if it's doing cross function
analysis correctly.
regards,
dan carpenter
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-01-26 7:59 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-01-26 6:50 [bug report] drm/i915/gvt/kvmgt: add vfio/mdev support to KVMGT Dan Carpenter
2017-01-26 7:21 ` Jike Song
2017-01-26 7:59 ` Dan Carpenter
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).