public inbox for linux-rdma@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] infiniband-diags/ibqueryerrors: Fix realloc size to prevent core
@ 2009-11-03 21:20 Ira Weiny
       [not found] ` <20091103132003.112e10b0.weiny2-i2BcT+NCU+M@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Ira Weiny @ 2009-11-03 21:20 UTC (permalink / raw)
  To: Sasha Khapyorsky; +Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org


From: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
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 <weiny2-i2BcT+NCU+M@public.gmane.org>
---
 infiniband-diags/src/ibqueryerrors.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/infiniband-diags/src/ibqueryerrors.c b/infiniband-diags/src/ibqueryerrors.c
index f83f29e..aac7087 100644
--- a/infiniband-diags/src/ibqueryerrors.c
+++ b/infiniband-diags/src/ibqueryerrors.c
@@ -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++;
 }
-- 
1.5.4.5

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] infiniband-diags/ibqueryerrors: Fix realloc size to prevent core
       [not found] ` <20091103132003.112e10b0.weiny2-i2BcT+NCU+M@public.gmane.org>
@ 2009-11-03 21:39   ` Al Chu
       [not found]     ` <1257284362.580.59.camel-X2zTWyBD0EhliZ7u+bvwcg@public.gmane.org>
  0 siblings, 1 reply; 5+ messages in thread
From: Al Chu @ 2009-11-03 21:39 UTC (permalink / raw)
  To: Ira Weiny
  Cc: Sasha Khapyorsky,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

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

On Tue, 2009-11-03 at 13:20 -0800, Ira Weiny wrote:
> From: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
> 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 <weiny2-i2BcT+NCU+M@public.gmane.org>
> ---
>  infiniband-diags/src/ibqueryerrors.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/infiniband-diags/src/ibqueryerrors.c b/infiniband-diags/src/ibqueryerrors.c
> index f83f29e..aac7087 100644
> --- a/infiniband-diags/src/ibqueryerrors.c
> +++ b/infiniband-diags/src/ibqueryerrors.c
> @@ -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++;
>  }
-- 
Albert Chu
chu11-i2BcT+NCU+M@public.gmane.org
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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH] infiniband-diags/ibqueryerrors: Fix realloc size to prevent core
       [not found]     ` <1257284362.580.59.camel-X2zTWyBD0EhliZ7u+bvwcg@public.gmane.org>
@ 2009-11-03 22:22       ` Sasha Khapyorsky
  2009-11-04  0:50         ` Ira Weiny
  2009-11-04  0:46       ` [PATCH V2] " Ira Weiny
  1 sibling, 1 reply; 5+ messages in thread
From: Sasha Khapyorsky @ 2009-11-03 22:22 UTC (permalink / raw)
  To: Al Chu; +Cc: Ira Weiny, linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

On 13:39 Tue 03 Nov     , Al Chu wrote:
> 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
> 
> On Tue, 2009-11-03 at 13:20 -0800, Ira Weiny wrote:
> > From: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
> > 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 <weiny2-i2BcT+NCU+M@public.gmane.org>
> > ---
> >  infiniband-diags/src/ibqueryerrors.c |    3 ++-
> >  1 files changed, 2 insertions(+), 1 deletions(-)
> > 
> > diff --git a/infiniband-diags/src/ibqueryerrors.c b/infiniband-diags/src/ibqueryerrors.c
> > index f83f29e..aac7087 100644
> > --- a/infiniband-diags/src/ibqueryerrors.c
> > +++ b/infiniband-diags/src/ibqueryerrors.c
> > @@ -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));

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

Sasha

> >  	suppressed_fields[sup_total] = field;
> >  	sup_total++;
> >  }
> -- 
> Albert Chu
> chu11-i2BcT+NCU+M@public.gmane.org
> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: [PATCH V2] infiniband-diags/ibqueryerrors: Fix realloc size to prevent core
       [not found]     ` <1257284362.580.59.camel-X2zTWyBD0EhliZ7u+bvwcg@public.gmane.org>
  2009-11-03 22:22       ` Sasha Khapyorsky
@ 2009-11-04  0:46       ` Ira Weiny
  1 sibling, 0 replies; 5+ messages in thread
From: Ira Weiny @ 2009-11-04  0:46 UTC (permalink / raw)
  To: Sasha Khapyorsky
  Cc: linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Al Chu

On Tue, 03 Nov 2009 13:39:22 -0800
Al Chu <chu11-i2BcT+NCU+M@public.gmane.org> wrote:

> Need to check for NULL?  Manpage says its possible:

Good idea; V2 below,
Ira


From: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
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 <weiny2-i2BcT+NCU+M@public.gmane.org>
---
 infiniband-diags/src/ibqueryerrors.c |    6 +++++-
 1 files changed, 5 insertions(+), 1 deletions(-)

diff --git a/infiniband-diags/src/ibqueryerrors.c b/infiniband-diags/src/ibqueryerrors.c
index f83f29e..2b5f63f 100644
--- a/infiniband-diags/src/ibqueryerrors.c
+++ b/infiniband-diags/src/ibqueryerrors.c
@@ -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++;
 }
-- 
1.5.4.5

--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH] infiniband-diags/ibqueryerrors: Fix realloc size to prevent core
  2009-11-03 22:22       ` Sasha Khapyorsky
@ 2009-11-04  0:50         ` Ira Weiny
  0 siblings, 0 replies; 5+ messages in thread
From: Ira Weiny @ 2009-11-04  0:50 UTC (permalink / raw)
  To: Sasha Khapyorsky
  Cc: Al Chu, Ira Weiny,
	linux-rdma-u79uwXL29TY76Z2rM5mHXA@public.gmane.org

On Wed, 4 Nov 2009 00:22:18 +0200
Sasha Khapyorsky <sashak-smomgflXvOZWk0Htik3J/w@public.gmane.org> wrote:

> On 13:39 Tue 03 Nov     , Al Chu wrote:
> > 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
> > 
> > On Tue, 2009-11-03 at 13:20 -0800, Ira Weiny wrote:
> > > From: Ira Weiny <weiny2-i2BcT+NCU+M@public.gmane.org>
> > > 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 <weiny2-i2BcT+NCU+M@public.gmane.org>
> > > ---
> > >  infiniband-diags/src/ibqueryerrors.c |    3 ++-
> > >  1 files changed, 2 insertions(+), 1 deletions(-)
> > > 
> > > diff --git a/infiniband-diags/src/ibqueryerrors.c b/infiniband-diags/src/ibqueryerrors.c
> > > index f83f29e..aac7087 100644
> > > --- a/infiniband-diags/src/ibqueryerrors.c
> > > +++ b/infiniband-diags/src/ibqueryerrors.c
> > > @@ -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));
> 
> 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

> 
> Sasha
> 
> > >  	suppressed_fields[sup_total] = field;
> > >  	sup_total++;
> > >  }
> > -- 
> > Albert Chu
> > chu11-i2BcT+NCU+M@public.gmane.org
> > 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 majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > More majordomo info at  http://*vger.kernel.org/majordomo-info.html
> > 
> 


-- 
Ira Weiny
Math Programmer/Computer Scientist
Lawrence Livermore National Lab
925-423-8008
weiny2-i2BcT+NCU+M@public.gmane.org
--
To unsubscribe from this list: send the line "unsubscribe linux-rdma" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2009-11-04  0:50 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-11-03 21:20 [PATCH] infiniband-diags/ibqueryerrors: Fix realloc size to prevent core Ira Weiny
     [not found] ` <20091103132003.112e10b0.weiny2-i2BcT+NCU+M@public.gmane.org>
2009-11-03 21:39   ` Al Chu
     [not found]     ` <1257284362.580.59.camel-X2zTWyBD0EhliZ7u+bvwcg@public.gmane.org>
2009-11-03 22:22       ` Sasha Khapyorsky
2009-11-04  0:50         ` Ira Weiny
2009-11-04  0:46       ` [PATCH V2] " Ira Weiny

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox