| Message ID | 20170722073335.tb6byaaarqvloh3a@mwanda (mailing list archive) |
|---|---|
| State | Not Applicable |
| Headers | show |
On Sat 7/22/2017 3:34 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote: > -----Original Message----- > From: Dan Carpenter [mailto:dan.carpenter@oracle.com] > Sent: Saturday, July 22, 2017 3:34 PM > To: Qiang Zhao <qiang.zhao@nxp.com> > Cc: Leo Li <leoyang.li@nxp.com>; linuxppc-dev@lists.ozlabs.org; kernel- > janitors@vger.kernel.org > Subject: [PATCH 1/2] fsl/qe: NULL dereference on error in ucc_of_parse_tdm() > > If "pdev = of_find_device_by_node(np2);" fails then it would lead to a NULL > dereference. This function is called from probe() and we're using managed > resources so we can just return without doing a manual cleanup. You mean it will be cleaned up automatically? > > Fixes: 35ef1c20fdb2 ("fsl/qe: Add QE TDM lib") > Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com> Best Regards Qiang Zhao
On Mon, Jul 24, 2017 at 02:24:14AM +0000, Qiang Zhao wrote: > On Sat 7/22/2017 3:34 PM, Dan Carpenter <dan.carpenter@oracle.com> wrote: > > > -----Original Message----- > > From: Dan Carpenter [mailto:dan.carpenter@oracle.com] > > Sent: Saturday, July 22, 2017 3:34 PM > > To: Qiang Zhao <qiang.zhao@nxp.com> > > Cc: Leo Li <leoyang.li@nxp.com>; linuxppc-dev@lists.ozlabs.org; kernel- > > janitors@vger.kernel.org > > Subject: [PATCH 1/2] fsl/qe: NULL dereference on error in ucc_of_parse_tdm() > > > > If "pdev = of_find_device_by_node(np2);" fails then it would lead to a NULL > > dereference. This function is called from probe() and we're using managed > > resources so we can just return without doing a manual cleanup. > > You mean it will be cleaned up automatically? Yes. At module unload. regards, dan carpenter
On Mon 7/24/2017 3:04 PM, Dan Carpenter <dan.carpenter@oracle.com> > -----Original Message----- > From: Dan Carpenter [mailto:dan.carpenter@oracle.com] > Sent: Monday, July 24, 2017 3:04 PM > To: Qiang Zhao <qiang.zhao@nxp.com> > Cc: Leo Li <leoyang.li@nxp.com>; linuxppc-dev@lists.ozlabs.org; kernel- > janitors@vger.kernel.org > Subject: Re: [PATCH 1/2] fsl/qe: NULL dereference on error in > ucc_of_parse_tdm() > > On Mon, Jul 24, 2017 at 02:24:14AM +0000, Qiang Zhao wrote: > > On Sat 7/22/2017 3:34 PM, Dan Carpenter <dan.carpenter@oracle.com> > wrote: > > > > > -----Original Message----- > > > From: Dan Carpenter [mailto:dan.carpenter@oracle.com] > > > Sent: Saturday, July 22, 2017 3:34 PM > > > To: Qiang Zhao <qiang.zhao@nxp.com> > > > Cc: Leo Li <leoyang.li@nxp.com>; linuxppc-dev@lists.ozlabs.org; > > > kernel- janitors@vger.kernel.org > > > Subject: [PATCH 1/2] fsl/qe: NULL dereference on error in > > > ucc_of_parse_tdm() > > > > > > If "pdev = of_find_device_by_node(np2);" fails then it would lead to > > > a NULL dereference. This function is called from probe() and we're > > > using managed resources so we can just return without doing a manual > cleanup. > > > > You mean it will be cleaned up automatically? > > Yes. At module unload. Do you mean when insmod it as a module, and when this module is removed, it will be cleaned up automatically? Do I understand correctly? Well, how about build-in? Best Regards Qiang Zhao
On Mon, Jul 24, 2017 at 09:39:32AM +0000, Qiang Zhao wrote: > On Mon 7/24/2017 3:04 PM, Dan Carpenter <dan.carpenter@oracle.com> > > > -----Original Message----- > > From: Dan Carpenter [mailto:dan.carpenter@oracle.com] > > Sent: Monday, July 24, 2017 3:04 PM > > To: Qiang Zhao <qiang.zhao@nxp.com> > > Cc: Leo Li <leoyang.li@nxp.com>; linuxppc-dev@lists.ozlabs.org; kernel- > > janitors@vger.kernel.org > > Subject: Re: [PATCH 1/2] fsl/qe: NULL dereference on error in > > ucc_of_parse_tdm() > > > > On Mon, Jul 24, 2017 at 02:24:14AM +0000, Qiang Zhao wrote: > > > On Sat 7/22/2017 3:34 PM, Dan Carpenter <dan.carpenter@oracle.com> > > wrote: > > > > > > > -----Original Message----- > > > > From: Dan Carpenter [mailto:dan.carpenter@oracle.com] > > > > Sent: Saturday, July 22, 2017 3:34 PM > > > > To: Qiang Zhao <qiang.zhao@nxp.com> > > > > Cc: Leo Li <leoyang.li@nxp.com>; linuxppc-dev@lists.ozlabs.org; > > > > kernel- janitors@vger.kernel.org > > > > Subject: [PATCH 1/2] fsl/qe: NULL dereference on error in > > > > ucc_of_parse_tdm() > > > > > > > > If "pdev = of_find_device_by_node(np2);" fails then it would lead to > > > > a NULL dereference. This function is called from probe() and we're > > > > using managed resources so we can just return without doing a manual > > cleanup. > > > > > > You mean it will be cleaned up automatically? > > > > Yes. At module unload. > > Do you mean when insmod it as a module, and when this module is removed, it will be cleaned up automatically? > Do I understand correctly? > Well, how about build-in? > Sorry, I mispoke. It's not really at module unload time. In this case it's removed automatically when probe() fails, but normally devm_ resources get released after the ->remove(). The devres_release_all() function is what releases these, so do a search for that function to see the details. regards, dan carpenter
diff --git a/drivers/soc/fsl/qe/qe_tdm.c b/drivers/soc/fsl/qe/qe_tdm.c index f744c214f680..ec7a853053c3 100644 --- a/drivers/soc/fsl/qe/qe_tdm.c +++ b/drivers/soc/fsl/qe/qe_tdm.c @@ -139,32 +139,25 @@ int ucc_of_parse_tdm(struct device_node *np, struct ucc_tdm *utdm, of_node_put(np2); res = platform_get_resource(pdev, IORESOURCE_MEM, 0); utdm->si_regs = devm_ioremap_resource(&pdev->dev, res); - if (IS_ERR(utdm->si_regs)) { - ret = PTR_ERR(utdm->si_regs); - goto err_miss_siram_property; - } + if (IS_ERR(utdm->si_regs)) + return PTR_ERR(utdm->si_regs); np2 = of_find_compatible_node(NULL, NULL, "fsl,t1040-qe-siram"); - if (!np2) { - ret = -EINVAL; - goto err_miss_siram_property; - } + if (!np2) + return -EINVAL; pdev = of_find_device_by_node(np2); if (!pdev) { - ret = -EINVAL; pr_err("%s: failed to lookup pdev\n", np2->name); of_node_put(np2); - goto err_miss_siram_property; + return -EINVAL; } of_node_put(np2); res = platform_get_resource(pdev, IORESOURCE_MEM, 0); utdm->siram = devm_ioremap_resource(&pdev->dev, res); - if (IS_ERR(utdm->siram)) { - ret = PTR_ERR(utdm->siram); - goto err_miss_siram_property; - } + if (IS_ERR(utdm->siram)) + return PTR_ERR(utdm->siram); if (siram_init_flag == 0) { memset_io(utdm->siram, 0, resource_size(res)); @@ -172,10 +165,6 @@ int ucc_of_parse_tdm(struct device_node *np, struct ucc_tdm *utdm, } return ret; - -err_miss_siram_property: - devm_iounmap(&pdev->dev, utdm->si_regs); - return ret; } EXPORT_SYMBOL(ucc_of_parse_tdm);
If "pdev = of_find_device_by_node(np2);" fails then it would lead to a NULL dereference. This function is called from probe() and we're using managed resources so we can just return without doing a manual cleanup. Fixes: 35ef1c20fdb2 ("fsl/qe: Add QE TDM lib") Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>