Skip to contentSkip to conversation

nfc: nci: fix potential NULL pointer dereference

PATCH In review Gustavo A. R. Silva 3 participants 8 messages Jun 12, 2017 – Jun 23, 2017

Subsystems (3)
Networking Network drivers NFC
lore
Share & tools
lore

Gustavo A. R. Silva

author
#0 lore raw

PATCH 1/1 nfc: nci: fix potential NULL pointer dereference

NULL check at line 76: if (conn_info) {, implies that pointer conn_info might be NULL, but this pointer is being previously dereferenced, which might cause a NULL pointer dereference.

Add NULL check before dereferencing pointer conn_info in order to avoid a potential NULL pointer dereference.

Addresses-Coverity-ID: 1362349

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 net/nfc/nci/core.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
net/nfc/nci/core.c +5 -6
index 61fff42..d2198ce 100644
@@ -70,14 +70,13 @@ int nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type,
 	struct nci_conn_info *conn_info;
 
 	list_for_each_entry(conn_info, &ndev->conn_info_list, list) {
-		if (conn_info->dest_type == dest_type) {
+		if (conn_info && conn_info->dest_type == dest_type) {
 			if (!params)
 				return conn_info->conn_id;
-			if (conn_info) {
-				if (params->id == conn_info->dest_params->id &&
-				    params->protocol == conn_info->dest_params->protocol)
-					return conn_info->conn_id;
-			}
+
+			if (params->id == conn_info->dest_params->id &&
+			    params->protocol == conn_info->dest_params->protocol)
+				return conn_info->conn_id;
 		}
 	}
Signature · 1 line
2.5.0
#1 lore raw
Quoted 18 lines · Gustavo A. R. Silva

NULL check at line 76: if (conn_info) {, implies that pointer conn_info might be NULL, but this pointer is being previously dereferenced, which might cause a NULL pointer dereference.

Add NULL check before dereferencing pointer conn_info in order to avoid a potential NULL pointer dereference.

Addresses-Coverity-ID: 1362349

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 net/nfc/nci/core.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
net/nfc/nci/core.c +0 -0
index 61fff42..d2198ce 100644

net/nfc/nci/core.c int nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type, · line 72

Quoted 3 lines
net/nfc/nci/core.c +0 -0
@@ -70,14 +70,13 @@ int nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type,
 	struct nci_conn_info *conn_info;
 
 	list_for_each_entry(conn_info, &ndev->conn_info_list, list) {

conn_info is set in list_for_each_entry() using container_of(), which is never NULL. Plus, it is dereferenced there as well. The check is unnecessary.

Guenter

Quoted 16 lines
net/nfc/nci/core.c +5 -6
-		if (conn_info->dest_type == dest_type) {
+		if (conn_info && conn_info->dest_type == dest_type) {
 			if (!params)
 				return conn_info->conn_id;
-			if (conn_info) {
-				if (params->id == conn_info->dest_params->id &&
-				    params->protocol == conn_info->dest_params->protocol)
-					return conn_info->conn_id;
-			}
+
+			if (params->id == conn_info->dest_params->id &&
+			    params->protocol == conn_info->dest_params->protocol)
+				return conn_info->conn_id;
 		}
 	}
author
#2 lore raw

Hi Guenter,

Please, see my comments below

Quoting Guenter Roeck <[email protected]>:

Quoted 22 lines · Gustavo A. R. Silva
Quoted 21 lines

NULL check at line 76: if (conn_info) {, implies that pointer conn_info might be NULL, but this pointer is being previously dereferenced, which might cause a NULL pointer dereference.

Add NULL check before dereferencing pointer conn_info in order to avoid a potential NULL pointer dereference.

Addresses-Coverity-ID: 1362349

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 net/nfc/nci/core.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
net/nfc/nci/core.c +0 -0
index 61fff42..d2198ce 100644
@@ -70,14 +70,13 @@ int  
 nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type,
 	struct nci_conn_info *conn_info;
Quoted 6 lines

net/nfc/nci/core.c int nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type, · line 72

Quoted 1 line
net/nfc/nci/core.c +0 -0
 	list_for_each_entry(conn_info, &ndev->conn_info_list, list) {

conn_info is set in list_for_each_entry() using container_of(), which is never NULL. Plus, it is dereferenced there as well. The check is unnecessary.

Thanks for clarifying.

Quoted 7 lines

Guenter

net/nfc/nci/core.c int nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type, · line 76

Quoted 5 lines
net/nfc/nci/core.c +1 -2
-		if (conn_info->dest_type == dest_type) {
+		if (conn_info && conn_info->dest_type == dest_type) {
 			if (!params)
 				return conn_info->conn_id;
-			if (conn_info) {

So, this NULL check could be removed as it seems it is not useful at all ?

Quoted 11 lines

net/nfc/nci/core.c int nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type, · line 81

Quoted 11 lines
net/nfc/nci/core.c +4 -4
-				if (params->id == conn_info->dest_params->id &&
-				    params->protocol == conn_info->dest_params->protocol)
-					return conn_info->conn_id;
-			}
+
+			if (params->id == conn_info->dest_params->id &&
+			    params->protocol == conn_info->dest_params->protocol)
+				return conn_info->conn_id;
 		}
 	}

Thank you

#3 lore raw
Quoted 38 lines · Gustavo A. R. Silva

Hi Guenter,

Please, see my comments below

Quoting Guenter Roeck <[email protected]>:

Quoted 21 lines · Gustavo A. R. Silva
Quoted 20 lines

NULL check at line 76: if (conn_info) {, implies that pointer conn_info might be NULL, but this pointer is being previously dereferenced, which might cause a NULL pointer dereference.

Add NULL check before dereferencing pointer conn_info in order to avoid a potential NULL pointer dereference.

Addresses-Coverity-ID: 1362349

Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 net/nfc/nci/core.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
net/nfc/nci/core.c +0 -0
index 61fff42..d2198ce 100644
@@ -70,14 +70,13 @@ int nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type,
     struct nci_conn_info *conn_info;
Quoted 6 lines

net/nfc/nci/core.c int nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type, · line 72

Quoted 1 line
net/nfc/nci/core.c +0 -0
     list_for_each_entry(conn_info, &ndev->conn_info_list, list) {

conn_info is set in list_for_each_entry() using container_of(), which is never NULL. Plus, it is dereferenced there as well. The check is unnecessary.

Thanks for clarifying.

Quoted 2 lines

Guenter

Quoted 8 lines
Quoted 5 lines

net/nfc/nci/core.c int nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type, · line 76

Quoted 5 lines
net/nfc/nci/core.c +1 -2
-        if (conn_info->dest_type == dest_type) {
+        if (conn_info && conn_info->dest_type == dest_type) {
             if (!params)
                 return conn_info->conn_id;
-            if (conn_info) {

So, this NULL check could be removed as it seems it is not useful at all ?

Exactly.

Quoted 22 lines
Quoted 11 lines

net/nfc/nci/core.c int nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type, · line 82

Quoted 11 lines
net/nfc/nci/core.c +4 -4
-                if (params->id == conn_info->dest_params->id &&
-                    params->protocol == conn_info->dest_params->protocol)
-                    return conn_info->conn_id;
-            }
+
+            if (params->id == conn_info->dest_params->id &&
+                params->protocol == conn_info->dest_params->protocol)
+                return conn_info->conn_id;
         }
     }

Thank you

Signature · 1 line
Gustavo A. R. Silva
author
#4 lore raw

PATCH 1/1 nfc: nci: remove unnecessary null check

Remove unnecessary NULL check for pointer conn_info. conn_info is set in list_for_each_entry() using container_of(), which is never NULL.

Addresses-Coverity-ID: 1362349

Cc Guenter Roeck <[email protected]>
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 net/nfc/nci/core.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
net/nfc/nci/core.c +4 -5
index 61fff42..c15cb88 100644
@@ -73,11 +73,10 @@ int nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type,
 		if (conn_info->dest_type == dest_type) {
 			if (!params)
 				return conn_info->conn_id;
-			if (conn_info) {
-				if (params->id == conn_info->dest_params->id &&
-				    params->protocol == conn_info->dest_params->protocol)
-					return conn_info->conn_id;
-			}
+
+			if (params->id == conn_info->dest_params->id &&
+			    params->protocol == conn_info->dest_params->protocol)
+				return conn_info->conn_id;
 		}
 	}
Signature · 1 line
2.5.0
Reviewed-by on v1 1/1
#5 lore raw

Re: [PATCH] nfc: nci: remove unnecessary null check

Quoted 7 lines · Gustavo A. R. Silva

Remove unnecessary NULL check for pointer conn_info. conn_info is set in list_for_each_entry() using container_of(), which is never NULL.

Addresses-Coverity-ID: 1362349

Cc Guenter Roeck <[email protected]>
Signed-off-by Gustavo A. R. Silva <[email protected]>
Reviewed-by Guenter Roeck <[email protected]>
Quoted 27 lines
---
 net/nfc/nci/core.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)
net/nfc/nci/core.c +4 -5
index 61fff42..c15cb88 100644
@@ -73,11 +73,10 @@ int nci_get_conn_info_by_dest_type_params(struct nci_dev *ndev, u8 dest_type,
 		if (conn_info->dest_type == dest_type) {
 			if (!params)
 				return conn_info->conn_id;
-			if (conn_info) {
-				if (params->id == conn_info->dest_params->id &&
-				    params->protocol == conn_info->dest_params->protocol)
-					return conn_info->conn_id;
-			}
+
+			if (params->id == conn_info->dest_params->id &&
+			    params->protocol == conn_info->dest_params->protocol)
+				return conn_info->conn_id;
 		}
 	}
Signature · 1 line
2.5.0
Applied
#6 lore raw

Re: [PATCH] nfc: nci: remove unnecessary null check

In reply to Gustavo A. R. Silva

Hi Gustavo,

Quoted 10 lines · Gustavo A. R. Silva

Remove unnecessary NULL check for pointer conn_info. conn_info is set in list_for_each_entry() using container_of(), which is never NULL.

Addresses-Coverity-ID: 1362349

Cc Guenter Roeck <[email protected]>
Signed-off-by Gustavo A. R. Silva <garsilva-L1vi/[email protected]>
---
 net/nfc/nci/core.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Applied to nfc-next, thanks.

Cheers,
Samuel.

author
#7 lore raw

Re: [PATCH] nfc: nci: remove unnecessary null check

Hi Samuel,

Quoting Samuel Ortiz <[email protected]>:

Quoted 7 lines

Hi Gustavo,

Quoted 4 lines · Gustavo A. R. Silva

Remove unnecessary NULL check for pointer conn_info. conn_info is set in list_for_each_entry() using container_of(), which is never NULL.

Quoted 8 lines
Quoted 6 lines

Addresses-Coverity-ID: 1362349

Cc Guenter Roeck <[email protected]>
Signed-off-by Gustavo A. R. Silva <[email protected]>
---
 net/nfc/nci/core.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

Applied to nfc-next, thanks.

Absolutely, glad to help.

Thanks