infiniband-diags/ibqueryerrors: Fix realloc size to prevent core

PATCH In review Ira Weiny 3 participants 5 messages Nov 3, 2009 – Nov 4, 2009 2 messages not in the archive InfiniBand

Share & tools

Ira Weiny

author

2009-11-03 21:20 UTC #0 lore raw

PATCH 1/1 infiniband-diags/ibqueryerrors: Fix realloc size to prevent core

From: Ira Weiny <[email protected]>
Date: Tue, 3 Nov 2009 13:19:33 -0800
Subject: [PATCH] infiniband-diags/ibqueryerrors: Fix realloc size to prevent core

Realloc size was wrong causing a core when enough errors were suppressed. Reproduced by running:

ibqueryerrors -c -s RcvSwRelayErrors,LinkDowned,VL15Dropped,XmtWait,SymbolErrors,LinkRecovers,RcvErrors

Signed-off-by Ira Weiny <[email protected]>

---
 infiniband-diags/src/ibqueryerrors.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

infiniband-diags/src/ibqueryerrors.c +2 -1

index f83f29e..aac7087 100644
@@ -381,7 +381,8 @@ void print_node(ibnd_node_t * node, void *user_data)
 
 static void add_suppressed(enum MAD_FIELDS field)
 {
-	suppressed_fields = realloc(suppressed_fields, sizeof(enum MAD_FIELDS));
+	suppressed_fields = realloc(suppressed_fields,
+				    (sup_total+1)*sizeof(enum MAD_FIELDS));
 	suppressed_fields[sup_total] = field;
 	sup_total++;
 }

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]

Al Chu

2009-11-03 21:39 UTC #2 lore raw

Need to check for NULL? Manpage says its possible:

---
realloc() returns a pointer to the newly allocated memory, which is suitably aligned for any kind of variable and may be different from ptr, or NULL if the request fails.
---

Quoted 28 lines · Ira Weiny

From: Ira Weiny <[email protected]>
Date: Tue, 3 Nov 2009 13:19:33 -0800
Subject: [PATCH] infiniband-diags/ibqueryerrors: Fix realloc size to prevent core
Realloc size was wrong causing a core when enough errors were suppressed. Reproduced by running:
ibqueryerrors -c -s RcvSwRelayErrors,LinkDowned,VL15Dropped,XmtWait,SymbolErrors,LinkRecovers,RcvErrors
Signed-off-by Ira Weiny <[email protected]>
---
 infiniband-diags/src/ibqueryerrors.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
infiniband-diags/src/ibqueryerrors.c +2 -1
index f83f29e..aac7087 100644
@@ -381,7 +381,8 @@ void print_node(ibnd_node_t * node, void *user_data)
 
 static void add_suppressed(enum MAD_FIELDS field)
 {
-	suppressed_fields = realloc(suppressed_fields, sizeof(enum MAD_FIELDS));
+	suppressed_fields = realloc(suppressed_fields,
+				    (sup_total+1)*sizeof(enum MAD_FIELDS));
 	suppressed_fields[sup_total] = field;
 	sup_total++;
 }

Signature · 10 lines

Albert Chu
[email protected]
Computer Scientist
High Performance Systems Division
Lawrence Livermore National Laboratory

--
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]

Sasha Khapyorsky

2009-11-03 22:22 UTC #4 lore raw

Quoted 31 lines · Al Chu

Need to check for NULL? Manpage says its possible:
---
realloc() returns a pointer to the newly allocated memory, which is suitably aligned for any kind of variable and may be different from ptr, or NULL if the request fails.
---
Al
Quoted 20 lines · Ira Weiny
From: Ira Weiny <[email protected]>
Date: Tue, 3 Nov 2009 13:19:33 -0800
Subject: [PATCH] infiniband-diags/ibqueryerrors: Fix realloc size to prevent core
Realloc size was wrong causing a core when enough errors were suppressed. Reproduced by running:
ibqueryerrors -c -s RcvSwRelayErrors,LinkDowned,VL15Dropped,XmtWait,SymbolErrors,LinkRecovers,RcvErrors
Signed-off-by Ira Weiny <[email protected]>
---
 infiniband-diags/src/ibqueryerrors.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
infiniband-diags/src/ibqueryerrors.c +0 -0
index f83f29e..aac7087 100644
@@ -381,7 +381,8 @@ void print_node(ibnd_node_t * node, void *user_data)
 

Quoted 5 lines

infiniband-diags/src/ibqueryerrors.c void print_node(ibnd_node_t * node, void *user_data) · line 385
Quoted 5 lines
infiniband-diags/src/ibqueryerrors.c +2 -1
 static void add_suppressed(enum MAD_FIELDS field)
 {
-	suppressed_fields = realloc(suppressed_fields, sizeof(enum MAD_FIELDS));
+	suppressed_fields = realloc(suppressed_fields,
+				    (sup_total+1)*sizeof(enum MAD_FIELDS));

Also it is never freed. Maybe just to have a static array (0-terminated) and to not bother with realloc()?

Sasha

Quoted 15 lines

Quoted 3 lines

infiniband-diags/src/ibqueryerrors.c +0 -0
 	suppressed_fields[sup_total] = field;
 	sup_total++;
 }

Signature · 10 lines

Albert Chu
[email protected]
Computer Scientist
High Performance Systems Division
Lawrence Livermore National Laboratory

--
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

--
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

Ira Weiny

author

2009-11-04 00:50 UTC #5 lore raw

Quoted 32 lines · On Wed, 4 Nov 2009 00:22:18 +0200 Sasha Khapyorsky…

Quoted 31 lines · Al Chu
Need to check for NULL? Manpage says its possible:
---
realloc() returns a pointer to the newly allocated memory, which is suitably aligned for any kind of variable and may be different from ptr, or NULL if the request fails.
---
Al
Quoted 20 lines · Ira Weiny
From: Ira Weiny <[email protected]>
Date: Tue, 3 Nov 2009 13:19:33 -0800
Subject: [PATCH] infiniband-diags/ibqueryerrors: Fix realloc size to prevent core
Realloc size was wrong causing a core when enough errors were suppressed. Reproduced by running:
ibqueryerrors -c -s RcvSwRelayErrors,LinkDowned,VL15Dropped,XmtWait,SymbolErrors,LinkRecovers,RcvErrors
Signed-off-by Ira Weiny <[email protected]>
---
 infiniband-diags/src/ibqueryerrors.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)
infiniband-diags/src/ibqueryerrors.c +0 -0
index f83f29e..aac7087 100644
@@ -381,7 +381,8 @@ void print_node(ibnd_node_t * node, void *user_data)
 

Quoted 8 lines

Quoted 5 lines
infiniband-diags/src/ibqueryerrors.c void print_node(ibnd_node_t * node, void *user_data) · line 385
Quoted 5 lines
infiniband-diags/src/ibqueryerrors.c +2 -1
 static void add_suppressed(enum MAD_FIELDS field)
 {
-	suppressed_fields = realloc(suppressed_fields, sizeof(enum MAD_FIELDS));
+	suppressed_fields = realloc(suppressed_fields,
+				    (sup_total+1)*sizeof(enum MAD_FIELDS));
Also it is never freed. Maybe just to have a static array (0-terminated) and to not bother with realloc()?

I thought we were not worrying about freeing things in the diags. I am all for doing proper clean up but there was a long thread on this where Hal was shot down for wanting everything free'ed properly.

<sigh> I will figure out how to make the static array later. Look for the patch.

Ira

Quoted 19 lines

Sasha

Quoted 15 lines

Quoted 3 lines

infiniband-diags/src/ibqueryerrors.c +0 -0
 	suppressed_fields[sup_total] = field;
 	sup_total++;
 }

Signature · 10 lines

Albert Chu
[email protected]
Computer Scientist
High Performance Systems Division
Lawrence Livermore National Laboratory

--
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

Signature · 9 lines

Ira Weiny
Math Programmer/Computer Scientist
Lawrence Livermore National Lab
925-423-8008
[email protected]
--
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

Ira Weiny

author

2009-11-04 00:46 UTC #6 lore raw

In reply to a message not in the archive

Quoted 1 line · On Tue, 03 Nov 2009 13:39:22 -0800 Al Chu <[email protected]>…

Need to check for NULL? Manpage says its possible:

Good idea; V2 below,
Ira

From: Ira Weiny <[email protected]>
Date: Tue, 3 Nov 2009 13:19:33 -0800
Subject: [PATCH] infiniband-diags/ibqueryerrors: Fix realloc size to prevent core

Realloc size was wrong causing a core when enough errors were suppressed. Reproduced by running:

ibqueryerrors -c -s RcvSwRelayErrors,LinkDowned,VL15Dropped,XmtWait,SymbolErrors,LinkRecovers,RcvErrors

Signed-off-by Ira Weiny <[email protected]>

---
 infiniband-diags/src/ibqueryerrors.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

infiniband-diags/src/ibqueryerrors.c +5 -1

index f83f29e..2b5f63f 100644
@@ -381,7 +381,11 @@ void print_node(ibnd_node_t * node, void *user_data)
 
 static void add_suppressed(enum MAD_FIELDS field)
 {
-	suppressed_fields = realloc(suppressed_fields, sizeof(enum MAD_FIELDS));
+	suppressed_fields = realloc(suppressed_fields,
+				    (sup_total+1)*sizeof(enum MAD_FIELDS));
+	if (!suppressed_fields)
+		IBERROR("realloc failed");
+
 	suppressed_fields[sup_total] = field;
 	sup_total++;
 }

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