| Message ID | 20221110150822.392385-1-yangyingliang@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | mailbox: zynq-ipi: fix error handling while device_register() fails | expand |
Thanks for your patch! This looks good to me. From 1860546acd4d860f8e00e469a3e5e3617017a8c2 Mon Sep 17 00:00:00 2001 From: Yang Yingliang <yangyingliang@huawei.com> Date: Thu, 10 Nov 2022 23:08:22 +0800 Subject: [PATCH] mailbox: zynq-ipi: fix error handling while device_register() fails > >If device_register() fails, it has two issues: >1. The name allocated by dev_set_name() is leaked. >2. The parent of device is not NULL, device_unregister() is called > in zynqmp_ipi_free_mboxes(), it will lead a kernel crash because > of removing not added device. > >Call put_device() to give up the reference, so the name is freed in >kobject_cleanup(). Add device registered check in zynqmp_ipi_free_mboxes() >to avoid null-ptr-deref. > >Fixes: 4981b82ba2ff ("mailbox: ZynqMP IPI mailbox controller") >Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >--- > drivers/mailbox/zynqmp-ipi-mailbox.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > >diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c >index 31a0fa914274..12e004ff1a14 100644 >--- a/drivers/mailbox/zynqmp-ipi-mailbox.c >+++ b/drivers/mailbox/zynqmp-ipi-mailbox.c >@@ -493,6 +493,7 @@ static int zynqmp_ipi_mbox_probe(struct zynqmp_ipi_mbox *ipi_mbox, > ret = device_register(&ipi_mbox->dev); > if (ret) { > dev_err(dev, "Failed to register ipi mbox dev.\n"); >+ put_device(&ipi_mbox->dev); > return ret; > } > mdev = &ipi_mbox->dev; >@@ -619,7 +620,8 @@ static void zynqmp_ipi_free_mboxes(struct zynqmp_ipi_pdata *pdata) > ipi_mbox = &pdata->ipi_mboxes[i]; > if (ipi_mbox->dev.parent) { > mbox_controller_unregister(&ipi_mbox->mbox); >- device_unregister(&ipi_mbox->dev); >+ if (device_is_registered(&ipi_mbox->dev)) >+ device_unregister(&ipi_mbox->dev); > } > } > } >
Reviewed-by: Tanmay Shah <tanmay.shah@amd.com> Thanks for your patch! This looks good to me. (Updated: Fixed Subject of the reply) >From: Yang Yingliang <yangyingliang@huawei.com> >Date: Thu, 10 Nov 2022 23:08:22 +0800 >Subject: [PATCH] mailbox: zynq-ipi: fix error handling while device_register() > fails > >If device_register() fails, it has two issues: >1. The name allocated by dev_set_name() is leaked. >2. The parent of device is not NULL, device_unregister() is called > in zynqmp_ipi_free_mboxes(), it will lead a kernel crash because > of removing not added device. > >Call put_device() to give up the reference, so the name is freed in >kobject_cleanup(). Add device registered check in zynqmp_ipi_free_mboxes() >to avoid null-ptr-deref. > >Fixes: 4981b82ba2ff ("mailbox: ZynqMP IPI mailbox controller") >Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> >--- > drivers/mailbox/zynqmp-ipi-mailbox.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > >diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c >index 31a0fa914274..12e004ff1a14 100644 >--- a/drivers/mailbox/zynqmp-ipi-mailbox.c >+++ b/drivers/mailbox/zynqmp-ipi-mailbox.c >@@ -493,6 +493,7 @@ static int zynqmp_ipi_mbox_probe(struct zynqmp_ipi_mbox *ipi_mbox, > ret = device_register(&ipi_mbox->dev); > if (ret) { > dev_err(dev, "Failed to register ipi mbox dev.\n"); >+ put_device(&ipi_mbox->dev); > return ret; > } > mdev = &ipi_mbox->dev; >@@ -619,7 +620,8 @@ static void zynqmp_ipi_free_mboxes(struct zynqmp_ipi_pdata *pdata) > ipi_mbox = &pdata->ipi_mboxes[i]; > if (ipi_mbox->dev.parent) { > mbox_controller_unregister(&ipi_mbox->mbox); >- device_unregister(&ipi_mbox->dev); >+ if (device_is_registered(&ipi_mbox->dev)) >+ device_unregister(&ipi_mbox->dev); > } > } > } > >-- >2.25.1
diff --git a/drivers/mailbox/zynqmp-ipi-mailbox.c b/drivers/mailbox/zynqmp-ipi-mailbox.c index 31a0fa914274..12e004ff1a14 100644 --- a/drivers/mailbox/zynqmp-ipi-mailbox.c +++ b/drivers/mailbox/zynqmp-ipi-mailbox.c @@ -493,6 +493,7 @@ static int zynqmp_ipi_mbox_probe(struct zynqmp_ipi_mbox *ipi_mbox, ret = device_register(&ipi_mbox->dev); if (ret) { dev_err(dev, "Failed to register ipi mbox dev.\n"); + put_device(&ipi_mbox->dev); return ret; } mdev = &ipi_mbox->dev; @@ -619,7 +620,8 @@ static void zynqmp_ipi_free_mboxes(struct zynqmp_ipi_pdata *pdata) ipi_mbox = &pdata->ipi_mboxes[i]; if (ipi_mbox->dev.parent) { mbox_controller_unregister(&ipi_mbox->mbox); - device_unregister(&ipi_mbox->dev); + if (device_is_registered(&ipi_mbox->dev)) + device_unregister(&ipi_mbox->dev); } } }
If device_register() fails, it has two issues: 1. The name allocated by dev_set_name() is leaked. 2. The parent of device is not NULL, device_unregister() is called in zynqmp_ipi_free_mboxes(), it will lead a kernel crash because of removing not added device. Call put_device() to give up the reference, so the name is freed in kobject_cleanup(). Add device registered check in zynqmp_ipi_free_mboxes() to avoid null-ptr-deref. Fixes: 4981b82ba2ff ("mailbox: ZynqMP IPI mailbox controller") Signed-off-by: Yang Yingliang <yangyingliang@huawei.com> --- drivers/mailbox/zynqmp-ipi-mailbox.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)