From: Xuan Zhuo <xuanzhuo@linux.alibaba.com> To: Wolfram Sang <wsa+renesas@sang-engineering.com> Cc: Wolfram Sang <wsa+renesas@sang-engineering.com>, "Michael S. Tsirkin" <mst@redhat.com>, Jason Wang <jasowang@redhat.com>, weiping zhang <zhangweiping@didichuxing.com>, Cornelia Huck <cohuck@redhat.com>, virtualization@lists.linux-foundation.org, linux-kernel@vger.kernel.org, linux-renesas-soc@vger.kernel.org Subject: Re: [PATCH] virtio-mmio: don't break lifecycle of vm_dev Date: Mon, 3 Jul 2023 10:11:37 +0800 [thread overview] Message-ID: <1688350297.9197447-5-xuanzhuo@linux.alibaba.com> (raw) In-Reply-To: <20230629120526.7184-1-wsa+renesas@sang-engineering.com> On Thu, 29 Jun 2023 14:05:26 +0200, Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > vm_dev has a separate lifecycle because it has a 'struct device' > embedded. Thus, having a release callback for it is correct. > > Allocating the vm_dev struct with devres totally breaks this protection, device? or driver? And why? > though. Instead of waiting for the vm_dev release callback, the memory > is freed when the platform_device is removed. Resulting in a > use-after-free when finally the callback is to be called. Can we have the break stack? Thanks. > > To easily see the problem, compile the kernel with > CONFIG_DEBUG_KOBJECT_RELEASE and unbind with sysfs. > > The fix is easy, don't use devres in this case. > > Found during my research about object lifetime problems. > > Fixes: 7eb781b1bbb7 ("virtio_mmio: add cleanup for virtio_mmio_probe") > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- > drivers/virtio/virtio_mmio.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c > index a46a4a29e929..97760f611295 100644 > --- a/drivers/virtio/virtio_mmio.c > +++ b/drivers/virtio/virtio_mmio.c > @@ -607,9 +607,8 @@ static void virtio_mmio_release_dev(struct device *_d) > struct virtio_device *vdev = > container_of(_d, struct virtio_device, dev); > struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); > - struct platform_device *pdev = vm_dev->pdev; > > - devm_kfree(&pdev->dev, vm_dev); > + kfree(vm_dev); > } > > /* Platform device */ > @@ -620,7 +619,7 @@ static int virtio_mmio_probe(struct platform_device *pdev) > unsigned long magic; > int rc; > > - vm_dev = devm_kzalloc(&pdev->dev, sizeof(*vm_dev), GFP_KERNEL); > + vm_dev = kzalloc(sizeof(*vm_dev), GFP_KERNEL); > if (!vm_dev) > return -ENOMEM; > > -- > 2.35.1 >
WARNING: multiple messages have this Message-ID (diff)
From: Xuan Zhuo <xuanzhuo@linux.alibaba.com> To: Wolfram Sang <wsa+renesas@sang-engineering.com> Cc: "Michael S. Tsirkin" <mst@redhat.com>, Cornelia Huck <cohuck@redhat.com>, linux-kernel@vger.kernel.org, virtualization@lists.linux-foundation.org, linux-renesas-soc@vger.kernel.org, Wolfram Sang <wsa+renesas@sang-engineering.com>, weiping zhang <zhangweiping@didichuxing.com> Subject: Re: [PATCH] virtio-mmio: don't break lifecycle of vm_dev Date: Mon, 3 Jul 2023 10:11:37 +0800 [thread overview] Message-ID: <1688350297.9197447-5-xuanzhuo@linux.alibaba.com> (raw) In-Reply-To: <20230629120526.7184-1-wsa+renesas@sang-engineering.com> On Thu, 29 Jun 2023 14:05:26 +0200, Wolfram Sang <wsa+renesas@sang-engineering.com> wrote: > vm_dev has a separate lifecycle because it has a 'struct device' > embedded. Thus, having a release callback for it is correct. > > Allocating the vm_dev struct with devres totally breaks this protection, device? or driver? And why? > though. Instead of waiting for the vm_dev release callback, the memory > is freed when the platform_device is removed. Resulting in a > use-after-free when finally the callback is to be called. Can we have the break stack? Thanks. > > To easily see the problem, compile the kernel with > CONFIG_DEBUG_KOBJECT_RELEASE and unbind with sysfs. > > The fix is easy, don't use devres in this case. > > Found during my research about object lifetime problems. > > Fixes: 7eb781b1bbb7 ("virtio_mmio: add cleanup for virtio_mmio_probe") > Signed-off-by: Wolfram Sang <wsa+renesas@sang-engineering.com> > --- > drivers/virtio/virtio_mmio.c | 5 ++--- > 1 file changed, 2 insertions(+), 3 deletions(-) > > diff --git a/drivers/virtio/virtio_mmio.c b/drivers/virtio/virtio_mmio.c > index a46a4a29e929..97760f611295 100644 > --- a/drivers/virtio/virtio_mmio.c > +++ b/drivers/virtio/virtio_mmio.c > @@ -607,9 +607,8 @@ static void virtio_mmio_release_dev(struct device *_d) > struct virtio_device *vdev = > container_of(_d, struct virtio_device, dev); > struct virtio_mmio_device *vm_dev = to_virtio_mmio_device(vdev); > - struct platform_device *pdev = vm_dev->pdev; > > - devm_kfree(&pdev->dev, vm_dev); > + kfree(vm_dev); > } > > /* Platform device */ > @@ -620,7 +619,7 @@ static int virtio_mmio_probe(struct platform_device *pdev) > unsigned long magic; > int rc; > > - vm_dev = devm_kzalloc(&pdev->dev, sizeof(*vm_dev), GFP_KERNEL); > + vm_dev = kzalloc(sizeof(*vm_dev), GFP_KERNEL); > if (!vm_dev) > return -ENOMEM; > > -- > 2.35.1 > _______________________________________________ Virtualization mailing list Virtualization@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
next prev parent reply other threads:[~2023-07-03 2:13 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-06-29 12:05 [PATCH] virtio-mmio: don't break lifecycle of vm_dev Wolfram Sang 2023-07-03 2:11 ` Xuan Zhuo [this message] 2023-07-03 2:11 ` Xuan Zhuo 2023-07-03 20:43 ` Wolfram Sang
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=1688350297.9197447-5-xuanzhuo@linux.alibaba.com \ --to=xuanzhuo@linux.alibaba.com \ --cc=cohuck@redhat.com \ --cc=jasowang@redhat.com \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-renesas-soc@vger.kernel.org \ --cc=mst@redhat.com \ --cc=virtualization@lists.linux-foundation.org \ --cc=wsa+renesas@sang-engineering.com \ --cc=zhangweiping@didichuxing.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.