Re: [dmidecode] [PATCH] dmioem: Fix segmentation fault in dmi_hp_240_attr()

[Jean Delvare]

On Thu, 4 Aug 2022 11:29:05 -0600, Jerry Hoemann wrote:
> On Thu, Aug 04, 2022 at 11:54:33AM +0200, Jean Delvare wrote:
> > pr_attr() does not accept a NULL format string. glibc can deal with
> > it, but FreeBSD's libc chokes on it.
> > 
> > Display the attributes as a list instead. Introduce pr_list_item_val()
> > to display list items with a value. This is currently implemented
> > exactly the same as pr_subattr(), but as the semantics are different,
> > I prefer to make it a separate function.
> > 
> > Bug reported by Scott Benesh (Microchip).
> > 
> > Signed-off-by: Jean Delvare <jdelvare@suse.de>
> > Fixes: a4b31b2bc537 ("dmioem: Present HPE type 240 attributes in a nicer 
> > way")
> > Cc: Jerry Hoemann <jerry.hoemann@hpe.com>  
> 
> Tested-by: Jerry Hoemann <jerry.hoemann@hpe.com>
> 
> Did you consider adding a check that format is nonzero to pr_attr()
> like pr_list_start() does?

Yes, I considered that first, especially as the other solution
duplicates code, which isn't appealing. However I decided against it
for semantic reasons.

> 
> Thanks
> 
> Jerry
> 
> > ---
> >  dmioem.c    |    5 +++--
> >  dmioutput.c |   12 ++++++++++++
> >  dmioutput.h |    1 +
> >  3 files changed, 16 insertions(+), 2 deletions(-)
> > 
> > --- dmidecode.orig/dmioem.c 2022-08-04 11:01:11.827321223 +0200
> > +++ dmidecode/dmioem.c      2022-08-04 11:48:36.927172122 +0200
> > @@ -198,13 +198,14 @@ static void dmi_hp_240_attr(u64 defined,
> >     };
> >     unsigned int i;
> >  
> > -   pr_attr("Attributes Defined/Set", NULL);
> > +   pr_list_start("Attributes Defined/Set", NULL);
> >     for (i = 0; i < ARRAY_SIZE(attributes); i++)
> >     {
> >             if (!(defined.l & (1UL << i)))
> >                     continue;
> > -           pr_subattr(attributes[i], "%s", set.l & (1UL << i) ? "Yes" : 
> > "No");
> > +           pr_list_item_val(attributes[i], "%s", set.l & (1UL << i) ? 
> > "Yes" : "No");
> >     }
> > +   pr_list_end();
> >  }
> >  
> >  static void dmi_hp_203_assoc_hndl(const char *fname, u16 num)
> > --- dmidecode.orig/dmioutput.c      2020-10-14 14:15:45.962471307 +0200
> > +++ dmidecode/dmioutput.c   2022-08-04 11:42:11.397632448 +0200
> > @@ -114,6 +114,18 @@ void pr_list_item(const char *format, ..
> >     printf("\n");
> >  }
> >  
> > +void pr_list_item_val(const char *name, const char *format, ...)
> > +{
> > +   va_list args;
> > +
> > +   printf("\t\t%s: ", name);
> > +
> > +   va_start(args, format);
> > +   vprintf(format, args);
> > +   va_end(args);
> > +   printf("\n");
> > +}
> > +
> >  void pr_list_end(void)
> >  {
> >     /* a no-op for text output */
> > --- dmidecode.orig/dmioutput.h      2020-10-14 14:15:45.962471307 +0200
> > +++ dmidecode/dmioutput.h   2022-08-04 11:40:19.178019967 +0200
> > @@ -29,6 +29,7 @@ void pr_attr(const char *name, const cha
> >  void pr_subattr(const char *name, const char *format, ...);
> >  void pr_list_start(const char *name, const char *format, ...);
> >  void pr_list_item(const char *format, ...);
> > +void pr_list_item_val(const char *name, const char *format, ...);
> >  void pr_list_end(void);
> >  void pr_sep(void);
> >  void pr_struct_err(const char *format, ...);
> > 
> > 
> > -- 
> > Jean Delvare
> > SUSE L3 Support  
> 


-- 
Jean Delvare
SUSE L3 Support