| Message ID | 20230517024651.82248-1-zhenzhong.duan@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [v2] vfio/pci: Fix a use-after-free issue | expand |
On 5/17/23 04:46, Zhenzhong Duan wrote: > vbasedev->name is freed wrongly which leads to garbage VFIO trace log. > Fix it by allocating a dup of vbasedev->name and then free the dup. > > Fixes: 2dca1b37a7 ("vfio/pci: add support for VF token") > Suggested-by: Alex Williamson <alex.williamson@redhat.com> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> Reviewed-by: Cédric Le Goater <clg@redhat.com> Thanks, C. > --- > v2: "toke" -> "token", Cedric > Update with Alex suggested change > > hw/vfio/pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index bf27a3990564..73874a94de12 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -2994,7 +2994,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp) > qemu_uuid_unparse(&vdev->vf_token, uuid); > name = g_strdup_printf("%s vf_token=%s", vbasedev->name, uuid); > } else { > - name = vbasedev->name; > + name = g_strdup(vbasedev->name); > } > > ret = vfio_get_device(group, name, vbasedev, errp);
On 5/16/23 10:46 PM, Zhenzhong Duan wrote: > vbasedev->name is freed wrongly which leads to garbage VFIO trace log. > Fix it by allocating a dup of vbasedev->name and then free the dup. > > Fixes: 2dca1b37a7 ("vfio/pci: add support for VF token") > Suggested-by: Alex Williamson <alex.williamson@redhat.com> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> Also verified that this resolves an issue seen on s390, as we were seeing not just garbage logs but QEMU crashes in certain cases e.g. during device unplug. Thanks! > --- > v2: "toke" -> "token", Cedric > Update with Alex suggested change > > hw/vfio/pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > index bf27a3990564..73874a94de12 100644 > --- a/hw/vfio/pci.c > +++ b/hw/vfio/pci.c > @@ -2994,7 +2994,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp) > qemu_uuid_unparse(&vdev->vf_token, uuid); > name = g_strdup_printf("%s vf_token=%s", vbasedev->name, uuid); > } else { > - name = vbasedev->name; > + name = g_strdup(vbasedev->name); > } > > ret = vfio_get_device(group, name, vbasedev, errp);
On Tue, 23 May 2023 13:00:53 -0400 Matthew Rosato <mjrosato@linux.ibm.com> wrote: > On 5/16/23 10:46 PM, Zhenzhong Duan wrote: > > vbasedev->name is freed wrongly which leads to garbage VFIO trace log. > > Fix it by allocating a dup of vbasedev->name and then free the dup. > > > > Fixes: 2dca1b37a7 ("vfio/pci: add support for VF token") > > Suggested-by: Alex Williamson <alex.williamson@redhat.com> > > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > > Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> > > Also verified that this resolves an issue seen on s390, as we were > seeing not just garbage logs but QEMU crashes in certain cases e.g. > during device unplug. Thanks! Thanks for the testing and reminder, I'll get a pull request out for this. Thanks, Alex > > --- > > v2: "toke" -> "token", Cedric > > Update with Alex suggested change > > > > hw/vfio/pci.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > > index bf27a3990564..73874a94de12 100644 > > --- a/hw/vfio/pci.c > > +++ b/hw/vfio/pci.c > > @@ -2994,7 +2994,7 @@ static void vfio_realize(PCIDevice *pdev, > > Error **errp) qemu_uuid_unparse(&vdev->vf_token, uuid); > > name = g_strdup_printf("%s vf_token=%s", vbasedev->name, > > uuid); } else { > > - name = vbasedev->name; > > + name = g_strdup(vbasedev->name); > > } > > > > ret = vfio_get_device(group, name, vbasedev, errp); >
On Tue, 23 May 2023 11:17:47 -0600 Alex Williamson <alex.williamson@redhat.com> wrote: > On Tue, 23 May 2023 13:00:53 -0400 > Matthew Rosato <mjrosato@linux.ibm.com> wrote: > > > On 5/16/23 10:46 PM, Zhenzhong Duan wrote: > > > vbasedev->name is freed wrongly which leads to garbage VFIO trace log. > > > Fix it by allocating a dup of vbasedev->name and then free the dup. > > > > > > Fixes: 2dca1b37a7 ("vfio/pci: add support for VF token") > > > Suggested-by: Alex Williamson <alex.williamson@redhat.com> > > > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > > > > Reviewed-by: Matthew Rosato <mjrosato@linux.ibm.com> > > > > Also verified that this resolves an issue seen on s390, as we were > > seeing not just garbage logs but QEMU crashes in certain cases e.g. > > during device unplug. Thanks! > > Thanks for the testing and reminder, I'll get a pull request out for > this. Thanks, Actually Cedric volunteered to bundle this with some pending patches, so... Acked-by: Alex Williamson <alex.williamson@redhat.com> > > > --- > > > v2: "toke" -> "token", Cedric > > > Update with Alex suggested change > > > > > > hw/vfio/pci.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c > > > index bf27a3990564..73874a94de12 100644 > > > --- a/hw/vfio/pci.c > > > +++ b/hw/vfio/pci.c > > > @@ -2994,7 +2994,7 @@ static void vfio_realize(PCIDevice *pdev, > > > Error **errp) qemu_uuid_unparse(&vdev->vf_token, uuid); > > > name = g_strdup_printf("%s vf_token=%s", vbasedev->name, > > > uuid); } else { > > > - name = vbasedev->name; > > > + name = g_strdup(vbasedev->name); > > > } > > > > > > ret = vfio_get_device(group, name, vbasedev, errp); > > >
On 17/5/23 04:46, Zhenzhong Duan wrote: > vbasedev->name is freed wrongly which leads to garbage VFIO trace log. > Fix it by allocating a dup of vbasedev->name and then free the dup. > > Fixes: 2dca1b37a7 ("vfio/pci: add support for VF token") > Suggested-by: Alex Williamson <alex.williamson@redhat.com> > Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> > --- > v2: "toke" -> "token", Cedric > Update with Alex suggested change > > hw/vfio/pci.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>
diff --git a/hw/vfio/pci.c b/hw/vfio/pci.c index bf27a3990564..73874a94de12 100644 --- a/hw/vfio/pci.c +++ b/hw/vfio/pci.c @@ -2994,7 +2994,7 @@ static void vfio_realize(PCIDevice *pdev, Error **errp) qemu_uuid_unparse(&vdev->vf_token, uuid); name = g_strdup_printf("%s vf_token=%s", vbasedev->name, uuid); } else { - name = vbasedev->name; + name = g_strdup(vbasedev->name); } ret = vfio_get_device(group, name, vbasedev, errp);
vbasedev->name is freed wrongly which leads to garbage VFIO trace log. Fix it by allocating a dup of vbasedev->name and then free the dup. Fixes: 2dca1b37a7 ("vfio/pci: add support for VF token") Suggested-by: Alex Williamson <alex.williamson@redhat.com> Signed-off-by: Zhenzhong Duan <zhenzhong.duan@intel.com> --- v2: "toke" -> "token", Cedric Update with Alex suggested change hw/vfio/pci.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)