[PATCH 1/2] fsl/qe: NULL dereference on error in ucc_of_parse_tdm()

Subsystems: freescale quicc engine library, freescale soc drivers, the rest

STALE3298d

6 messages, 2 authors, 2017-07-24 · open the first message on its own page

[PATCH 1/2] fsl/qe: NULL dereference on error in ucc_of_parse_tdm()

From: Dan Carpenter <hidden>
Date: 2017-07-22 07:33:51

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 <redacted>
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);
 

[PATCH 2/2] fsl/qe: Cleanup error paths in ucc_of_parse_tdm()

From: Dan Carpenter <hidden>
Date: 2017-07-22 07:36:31

The most important part of this change is that it not propogates error
codes instead of returning -EINVAL.  There was also a tab missing, and
a couple other minor cleanups which don't affect runtime.

Signed-off-by: Dan Carpenter <redacted>
---
Not tested, but I don't think propogating the errors will cause a
problem.
diff --git a/drivers/soc/fsl/qe/qe_tdm.c b/drivers/soc/fsl/qe/qe_tdm.c
index ec7a853053c3..ce071a78209b 100644
--- a/drivers/soc/fsl/qe/qe_tdm.c
+++ b/drivers/soc/fsl/qe/qe_tdm.c
@@ -42,7 +42,7 @@ int ucc_of_parse_tdm(struct device_node *np, struct ucc_tdm *utdm,
 		     struct ucc_tdm_info *ut_info)
 {
 	const char *sprop;
-	int ret = 0;
+	int ret;
 	u32 val;
 	struct resource *res;
 	struct device_node *np2;
@@ -68,7 +68,7 @@ int ucc_of_parse_tdm(struct device_node *np, struct ucc_tdm *utdm,
 		if ((ut_info->uf_info.tx_sync < QE_CLK_NONE) ||
 		    (ut_info->uf_info.tx_sync > QE_TSYNC_PIN)) {
 			pr_err("QE-TDM: Invalid tx-sync-clock property\n");
-		return -EINVAL;
+			return -EINVAL;
 		}
 	} else {
 		pr_err("QE-TDM: Invalid tx-sync-clock property\n");
@@ -78,13 +78,12 @@ int ucc_of_parse_tdm(struct device_node *np, struct ucc_tdm *utdm,
 	ret = of_property_read_u32_index(np, "fsl,tx-timeslot-mask", 0, &val);
 	if (ret) {
 		pr_err("QE-TDM: Invalid tx-timeslot-mask property\n");
-		return -EINVAL;
+		return ret;
 	}
 	utdm->tx_ts_mask = val;
 
 	ret = of_property_read_u32_index(np, "fsl,rx-timeslot-mask", 0, &val);
 	if (ret) {
-		ret = -EINVAL;
 		pr_err("QE-TDM: Invalid rx-timeslot-mask property\n");
 		return ret;
 	}
@@ -92,7 +91,6 @@ int ucc_of_parse_tdm(struct device_node *np, struct ucc_tdm *utdm,
 
 	ret = of_property_read_u32_index(np, "fsl,tdm-id", 0, &val);
 	if (ret) {
-		ret = -EINVAL;
 		pr_err("QE-TDM: No fsl,tdm-id property for this UCC\n");
 		return ret;
 	}
@@ -106,18 +104,16 @@ int ucc_of_parse_tdm(struct device_node *np, struct ucc_tdm *utdm,
 
 	sprop = of_get_property(np, "fsl,tdm-framer-type", NULL);
 	if (!sprop) {
-		ret = -EINVAL;
 		pr_err("QE-TDM: No tdm-framer-type property for UCC\n");
-		return ret;
+		return -EINVAL;
 	}
 	ret = set_tdm_framer(sprop);
 	if (ret < 0)
-		return -EINVAL;
+		return ret;
 	utdm->tdm_framer_type = ret;
 
 	ret = of_property_read_u32_index(np, "fsl,siram-entry-id", 0, &val);
 	if (ret) {
-		ret = -EINVAL;
 		pr_err("QE-TDM: No siram entry id for UCC\n");
 		return ret;
 	}
@@ -164,7 +160,7 @@ int ucc_of_parse_tdm(struct device_node *np, struct ucc_tdm *utdm,
 		siram_init_flag = 1;
 	}
 
-	return ret;
+	return 0;
 }
 EXPORT_SYMBOL(ucc_of_parse_tdm);
 

RE: [PATCH 1/2] fsl/qe: NULL dereference on error in ucc_of_parse_tdm()

From: Qiang Zhao <qiang.zhao@nxp.com>
Date: 2017-07-24 02:24:22

On Sat 7/22/2017 3:34 PM, Dan Carpenter [off-list ref] 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 <redacted>; linuxppc-dev@lists.ozlabs.org; kernel-
janitors@vger.kernel.org
Subject: [PATCH 1/2] fsl/qe: NULL dereference on error in ucc_of_parse_td=
m()
=20
If "pdev =3D 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 manage=
d
resources so we can just return without doing a manual cleanup.
You mean it will be cleaned up automatically?
=20
Fixes: 35ef1c20fdb2 ("fsl/qe: Add QE TDM lib")
Signed-off-by: Dan Carpenter <redacted>
Best Regards
Qiang Zhao

Re: [PATCH 1/2] fsl/qe: NULL dereference on error in ucc_of_parse_tdm()

From: Dan Carpenter <hidden>
Date: 2017-07-24 07:04:42

On Mon, Jul 24, 2017 at 02:24:14AM +0000, Qiang Zhao wrote:
On Sat 7/22/2017 3:34 PM, Dan Carpenter [off-list ref] wrote:
quoted
-----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 <redacted>; 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

RE: [PATCH 1/2] fsl/qe: NULL dereference on error in ucc_of_parse_tdm()

From: Qiang Zhao <qiang.zhao@nxp.com>
Date: 2017-07-24 09:39:41

On Mon 7/24/2017 3:04 PM, Dan Carpenter [off-list ref]
-----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 <redacted>; 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()
=20
On Mon, Jul 24, 2017 at 02:24:14AM +0000, Qiang Zhao wrote:
quoted
On Sat 7/22/2017 3:34 PM, Dan Carpenter [off-list ref]
wrote:
quoted
quoted
-----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 <redacted>; 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 =3D of_find_device_by_node(np2);" fails then it would lead t=
o
quoted
quoted
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.
quoted
You mean it will be cleaned up automatically?
=20
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

Re: [PATCH 1/2] fsl/qe: NULL dereference on error in ucc_of_parse_tdm()

From: Dan Carpenter <hidden>
Date: 2017-07-24 09:51:41

On Mon, Jul 24, 2017 at 09:39:32AM +0000, Qiang Zhao wrote:
On Mon 7/24/2017 3:04 PM, Dan Carpenter [off-list ref]
quoted
-----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 <redacted>; 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:
quoted
On Sat 7/22/2017 3:34 PM, Dan Carpenter [off-list ref]
wrote:
quoted
quoted
-----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 <redacted>; 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.
quoted
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
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help