Skip to contentSkip to conversation

infiniband-diags: Verify timeout value specified to diagnostics

PATCH Applied Jay Fenlason 3 participants 3 messages Dec 20, 2010 – Jan 2, 2011 2 messages not in the archive InfiniBand

lore
Share & tools
lore

Jay Fenlason

author
#0 lore raw

Sorry if you get this more than once, I seem to have outgoing mail misconfiguration here.

infiniband-diags/src/ibdia

Quoted 10 lines · Ira Weiny

Verify timeout value specified to diagnostics

Signed-off-by Ira Weiny <[email protected]>
---
 infiniband-diags/src/ibdiag_common.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)
infiniband-diags/src/ibdia renamed from infiniband-diags/src/ibdiag_common.c +0 -0

+g_common.c

Quoted 21 lines

index 99861f1..8ccf2fc 100644

infiniband-diags/src/ibdiag_common.c +7 -4
@@ -175,9 +175,13 @@ static int process_opt(int ch, char *optarg)
              ibd_dest_type = IB_DEST_GUID;
              break;
      case 't':
-             val = strtoul(optarg, 0, 0);
-             madrpc_set_timeout(val);
-             ibd_timeout = val;
+             val = (int)strtol(optarg, NULL, 0);
+             if (val > 0) {
+                     madrpc_set_timeout(val);
+                     ibd_timeout = val;
+             } else
+                     IBERROR("Invalid timeout \"%s\".  Timeout requires a "
+                             "positive integer value.", optarg);
              break;
      case 's':
              /* srcport is not required when resolving via IB_DEST_LID */
--
Quoted 1 line

1.5.4.5

This only partially detects invalid timeouts. For example, timeouts of "123skidoo" or 1234534587347895789457897897894578978912902393 will be accepted but will not work as expected. To fully test the conversion, you need to do something more like:

long val;
char *endp;

  ...
  errno = 0;
  val = strtol ( optarg, &endp 0 );
  if ( errno || ( endp && *endp != '\0' ) || val < 0 || val > INT_MAX ) {
        /* invalid timeout detected */
  ...

All of the tests are required to detect invalid inputs.

		-- JF

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

#2 lore raw

PATCH 1/1 Further timeout paramater verification (Was: [PATCH] infiniband-diags: Verify timeout value specified to diagnostics)

Quoted 52 lines · On Mon, 20 Dec 2010 13:57:36 -0800 Jay Fenlason…

Sorry if you get this more than once, I seem to have outgoing mail misconfiguration here.

infiniband-diags/src/ibdia

Quoted 10 lines · Ira Weiny

Verify timeout value specified to diagnostics

Signed-off-by Ira Weiny <[email protected]>
---
 infiniband-diags/src/ibdiag_common.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)
infiniband-diags/src/ibdia renamed from infiniband-diags/src/ibdiag_common.c +0 -0

+g_common.c

Quoted 21 lines

index 99861f1..8ccf2fc 100644

infiniband-diags/src/ibdiag_common.c +7 -4
@@ -175,9 +175,13 @@ static int process_opt(int ch, char *optarg)
              ibd_dest_type = IB_DEST_GUID;
              break;
      case 't':
-             val = strtoul(optarg, 0, 0);
-             madrpc_set_timeout(val);
-             ibd_timeout = val;
+             val = (int)strtol(optarg, NULL, 0);
+             if (val > 0) {
+                     madrpc_set_timeout(val);
+                     ibd_timeout = val;
+             } else
+                     IBERROR("Invalid timeout \"%s\".  Timeout requires a "
+                             "positive integer value.", optarg);
              break;
      case 's':
              /* srcport is not required when resolving via IB_DEST_LID */
--
Quoted 1 line

1.5.4.5

This only partially detects invalid timeouts. For example, timeouts of "123skidoo" or 1234534587347895789457897897894578978912902393 will be accepted but will not work as expected. To fully test the conversion, you need to do something more like:

long val;
char *endp;

  ...
  errno = 0;
  val = strtol ( optarg, &endp 0 );
  if ( errno || ( endp && *endp != '\0' ) || val < 0 || val > INT_MAX ) {
        /* invalid timeout detected */
  ...
Quoted 3 lines

All of the tests are required to detect invalid inputs.

		-- JF

Patch below,
Ira

From: Ira Weiny <[email protected]>
Date: Wed, 22 Dec 2010 11:40:33 -0800
Subject: [PATCH] Further timeout paramater verification

suggested by Jay Fenlason <[email protected]>

Signed-off-by Ira Weiny <[email protected]>
---
 infiniband-diags/src/ibdiag_common.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)
infiniband-diags/src/ibdiag_common.c +11 -7
index 8ccf2fc..8f5eec3 100644
@@ -138,7 +138,8 @@ void ibdiag_show_usage()
 
 static int process_opt(int ch, char *optarg)
 {
-	int val;
+	char *endp;
+	long val;
 
 	switch (ch) {
 	case 'h':

@@ -175,13 +176,16 @@ static int process_opt(int ch, char *optarg)
 		ibd_dest_type = IB_DEST_GUID;
 		break;
 	case 't':
-		val = (int)strtol(optarg, NULL, 0);
-		if (val > 0) {
-			madrpc_set_timeout(val);
-			ibd_timeout = val;
-		} else
+		errno = 0;
+		val = strtol(optarg, &endp, 0);
+		if (errno || (endp && *endp != '\0') || val <= 0 ||
+		    val > INT_MAX)
 			IBERROR("Invalid timeout \"%s\".  Timeout requires a "
-				"positive integer value.", optarg);
+				"positive integer value < %d.", optarg, INT_MAX);
+		else {
+			madrpc_set_timeout((int)val);
+			ibd_timeout = (int)val;
+		}
 		break;
 	case 's':
 		/* srcport is not required when resolving via IB_DEST_LID */
Signature · 6 lines
1.5.4.5

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

A message that is not in the archive (sent off-list or not yet received): [email protected]

Applied
#4 lore raw

Re: [PATCH] Further timeout paramater verification (Was: [PATCH] infiniband-diags: Verify timeout value specified to diagnostics)

Quoted 92 lines · Ira Weiny
Quoted 52 lines · On Mon, 20 Dec 2010 13:57:36 -0800 Jay Fenlason…

Sorry if you get this more than once, I seem to have outgoing mail misconfiguration here.

infiniband-diags/src/ibdia

Quoted 10 lines · Ira Weiny

Verify timeout value specified to diagnostics

Signed-off-by Ira Weiny <[email protected]>
---
 infiniband-diags/src/ibdiag_common.c |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)
infiniband-diags/src/ibdia renamed from infiniband-diags/src/ibdiag_common.c +0 -0

+g_common.c

Quoted 21 lines

index 99861f1..8ccf2fc 100644

infiniband-diags/src/ibdiag_common.c +7 -4
@@ -175,9 +175,13 @@ static int process_opt(int ch, char *optarg)
              ibd_dest_type = IB_DEST_GUID;
              break;
      case 't':
-             val = strtoul(optarg, 0, 0);
-             madrpc_set_timeout(val);
-             ibd_timeout = val;
+             val = (int)strtol(optarg, NULL, 0);
+             if (val > 0) {
+                     madrpc_set_timeout(val);
+                     ibd_timeout = val;
+             } else
+                     IBERROR("Invalid timeout \"%s\".  Timeout requires a "
+                             "positive integer value.", optarg);
              break;
      case 's':
              /* srcport is not required when resolving via IB_DEST_LID */
--
Quoted 1 line

1.5.4.5

This only partially detects invalid timeouts. For example, timeouts of "123skidoo" or 1234534587347895789457897897894578978912902393 will be accepted but will not work as expected. To fully test the conversion, you need to do something more like:

long val;
char *endp;

  ...
  errno = 0;
  val = strtol ( optarg, &endp 0 );
  if ( errno || ( endp && *endp != '\0' ) || val < 0 || val > INT_MAX ) {
        /* invalid timeout detected */
  ...
Quoted 3 lines

All of the tests are required to detect invalid inputs.

		-- JF

Patch below,
Ira

From: Ira Weiny <[email protected]>
Date: Wed, 22 Dec 2010 11:40:33 -0800
Subject: [PATCH] Further timeout paramater verification

suggested by Jay Fenlason <[email protected]>

Signed-off-by Ira Weiny <[email protected]>
---
 infiniband-diags/src/ibdiag_common.c |   18 +++++++++++-------
 1 files changed, 11 insertions(+), 7 deletions(-)
infiniband-diags/src/ibdiag_common.c +2 -1
index 8ccf2fc..8f5eec3 100644
@@ -138,7 +138,8 @@ void ibdiag_show_usage()
 
 static int process_opt(int ch, char *optarg)
 {
-	int val;
+	char *endp;
+	long val;
 
 	switch (ch) {
 	case 'h':

@@ -175,13 +176,16 @@ static int process_opt(int ch, char *optarg)
 		ibd_dest_type = IB_DEST_GUID;
 		break;

infiniband-diags/src/ibdiag_common.c static int process_opt(int ch, char *optarg) · line 182

Quoted 10 lines
infiniband-diags/src/ibdiag_common.c +4 -5
 	case 't':
-		val = (int)strtol(optarg, NULL, 0);
-		if (val > 0) {
-			madrpc_set_timeout(val);
-			ibd_timeout = val;
-		} else
+		errno = 0;
+		val = strtol(optarg, &endp, 0);
+		if (errno || (endp && *endp != '\0') || val <= 0 ||
+		    val > INT_MAX)

When using INT_MAX <limits.h> header should be included to prevent

src/ibdiag_common.c:182: error: ‘INT_MAX’ undeclared

error. I'm fixing this.

Applied. Thanks.

Sasha

Quoted 13 lines
infiniband-diags/src/ibdiag_common.c +5 -1
 			IBERROR("Invalid timeout \"%s\".  Timeout requires a "
-				"positive integer value.", optarg);
+				"positive integer value < %d.", optarg, INT_MAX);
+		else {
+			madrpc_set_timeout((int)val);
+			ibd_timeout = (int)val;
+		}
 		break;
 	case 's':
 		/* srcport is not required when resolving via IB_DEST_LID */
Signature · 1 line
1.5.4.5

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html