On 07:11 Tue 31 Oct     , Michael S. Tsirkin wrote:
> Quoting r. Sasha Khapyorsky <[EMAIL PROTECTED]>:
> > Subject: Re: [PATCH] diags/saquery: fix node_desc.description as string 
> > usages
> > 
> > On 06:30 Tue 31 Oct     , Michael S. Tsirkin wrote:
> > > Quoting r. Sasha Khapyorsky <[EMAIL PROTECTED]>:
> > > > Subject: Re: [PATCH] diags/saquery: fix node_desc.description as string 
> > > > usages
> > > > 
> > > > On 13:44 Mon 30 Oct     , Michael S. Tsirkin wrote:
> > > > > Quoting r. Sasha Khapyorsky <[EMAIL PROTECTED]>:
> > > > > > Subject: [PATCH] diags/saquery: fix node_desc.description as string 
> > > > > > usages
> > > > > > 
> > > > > > 
> > > > > > node_desc.description buffer is received from the network and should
> > > > > > not be NULL-terminated. In such cases using it as regular string in
> > > > > > functions like strcmp() or printf() leads to segmentation faults.
> > > > > > This patch fixes such usages.
> > > > > > 
> > > > > > Signed-off-by: Sasha Khapyorsky <[EMAIL PROTECTED]>
> > > > > > ---
> > > > > >  diags/src/saquery.c |   22 ++++++++++++++++------
> > > > > >  1 files changed, 16 insertions(+), 6 deletions(-)
> > > > > > 
> > > > > > diff --git a/diags/src/saquery.c b/diags/src/saquery.c
> > > > > > index 5b4a85e..f5b23fd 100644
> > > > > > --- a/diags/src/saquery.c
> > > > > > +++ b/diags/src/saquery.c
> > > > > > @@ -90,17 +90,21 @@ static void
> > > > > >  print_node_desc(ib_node_record_t *node_record)
> > > > > >  {
> > > > > >     ib_node_info_t *p_ni = &(node_record->node_info);
> > > > > > +   ib_node_desc_t *p_nd = &(node_record->node_desc);
> > > > > >     if (p_ni->node_type == IB_NODE_TYPE_CA)
> > > > > >     {
> > > > > > +           char desc[sizeof(p_nd->description) + 1];
> > > > > > +           memcpy(desc, p_nd->description, 
> > > > > > sizeof(p_nd->description));
> > > > > > +           desc[sizeof(desc) - 1] = '\0';
> > > > > >             printf("%6d  \"%s\"\n",
> > > > > > -                  cl_ntoh16(node_record->lid),
> > > > > > -                  node_record->node_desc.description);
> > > > > > +                  cl_ntoh16(node_record->lid), desc);
> > > > > >     }
> > > > > >  }
> > > > > 
> > > > > Would it not be simpler, and cleaner, to limit the string width in 
> > > > > printf:
> > > > >               printf("%6d  \"%.*s\"\n",
> > > > >                      cl_ntoh16(node_record->lid),
> > > > >                       sizeof(desc),
> > > > >                       node_record->node_desc.description);
> > > > 
> > > > This would be simpler. However some web searching shows that not all
> > > > printf() implementation permits not null terminated arrays even when
> > > > precision is specified (some issues were reported even with 
> > > > glibc-2.3.2).
> > > 
> > > Hmm, couldn't find it.
> > 
> > Look at this for example:
> > http://sourceware.org/ml/bug-glibc/2005-02/msg00123.html
> 
> Hmm, yea. Do you understand the answer there?
> Does not make sense to me ...

I less care about the answer, but more about valgrind output (btw
cannot see this with glibc-2.5) and similar "corner case" issues.

Sasha

_______________________________________________
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general

To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general

Reply via email to