diff mbox

[U-Boot,v3,3/3] net: eth: Check return value in various places

Message ID 1444279544-31914-3-git-send-email-bmeng.cn@gmail.com
State Accepted
Delegated to: Joe Hershberger
Headers show

Commit Message

Bin Meng Oct. 8, 2015, 4:45 a.m. UTC 
eth_get_dev() can return NULL which means device_probe() fails for
that ethernet device. Add return value check in various places or
U-Boot will crash due to NULL pointer access.

With this commit, 'dm_test_eth_act' test case passes.

Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
Acked-by: Joe Hershberger <joe.hershberger@ni.com>

---

Changes in v3: None
Changes in v2:
- Change check logic in eth_init() (do not use break)
- Add device_probe() return value check

 net/eth.c | 43 +++++++++++++++++++++++++------------------
 1 file changed, 25 insertions(+), 18 deletions(-)

Comments

Joe Hershberger Oct. 29, 2015, 7:31 p.m. UTC | #1 
On Wed, Oct 7, 2015 at 11:45 PM, Bin Meng <bmeng.cn@gmail.com> wrote:
> eth_get_dev() can return NULL which means device_probe() fails for
> that ethernet device. Add return value check in various places or
> U-Boot will crash due to NULL pointer access.
>
> With this commit, 'dm_test_eth_act' test case passes.
>
> Signed-off-by: Bin Meng <bmeng.cn@gmail.com>
> Acked-by: Joe Hershberger <joe.hershberger@ni.com>

Applied to u-boot-net/master, thanks!
-Joe
diff mbox

Patch

diff --git a/net/eth.c b/net/eth.c
index 0d66f33..a754651 100644
--- a/net/eth.c
+++ b/net/eth.c
@@ -179,8 +179,12 @@  struct udevice *eth_get_dev(void)
  */
 static void eth_set_dev(struct udevice *dev)
 {
-	if (dev && !device_active(dev))
+	if (dev && !device_active(dev)) {
 		eth_errno = device_probe(dev);
+		if (eth_errno)
+			dev = NULL;
+	}
+
 	eth_get_uclass_priv()->current = dev;
 }
 
@@ -213,10 +217,9 @@  struct udevice *eth_get_dev_by_name(const char *devname)
 		 * match an alias or it will match a literal name and we'll pick
 		 * up the error when we try to probe again in eth_set_dev().
 		 */
-		device_probe(it);
-		/*
-		 * Check for the name or the sequence number to match
-		 */
+		if (device_probe(it))
+			continue;
+		/* Check for the name or the sequence number to match */
 		if (strcmp(it->name, devname) == 0 ||
 		    (endp > startp && it->seq == seq))
 			return it;
@@ -346,23 +349,27 @@  int eth_init(void)
 
 	old_current = current;
 	do {
-		debug("Trying %s\n", current->name);
-
-		if (device_active(current)) {
-			ret = eth_get_ops(current)->start(current);
-			if (ret >= 0) {
-				struct eth_device_priv *priv =
-					current->uclass_priv;
-
-				priv->state = ETH_STATE_ACTIVE;
-				return 0;
+		if (current) {
+			debug("Trying %s\n", current->name);
+
+			if (device_active(current)) {
+				ret = eth_get_ops(current)->start(current);
+				if (ret >= 0) {
+					struct eth_device_priv *priv =
+						current->uclass_priv;
+
+					priv->state = ETH_STATE_ACTIVE;
+					return 0;
+				}
+			} else {
+				ret = eth_errno;
 			}
+
+			debug("FAIL\n");
 		} else {
-			ret = eth_errno;
+			debug("PROBE FAIL\n");
 		}
 
-		debug("FAIL\n");
-
 		/*
 		 * If ethrotate is enabled, this will change "current",
 		 * otherwise we will drop out of this while loop immediately