Re: [LTP] [PATCH v2] tst_kconfig: Avoid reporting buffer overflow when parsing /proc/cmdline

[Petr Vorel <pvorel@suse.cz>]

>  Li Wang <liwang@redhat.com> wrote:

> When the test is run with a kernel booting with many parameters, the
> > buffer size is often not large enough to store the complete command
> > line. This results in a buffer overflow and the test complains with
> > the following message:

> >   tst_kconfig.c:609: TWARN: Buffer overflowed while parsing /proc/cmdline

> > Note:

> > Petr point out that these configs, which are generated by toolchain will
> > be longer than 128 chars someday, but I don't think that is the reason
> > we need raise our parsed buffer, since tst_kcmdline_parse() was just add
> > for popular parameter (which always pass by user and short). So far I
> > don't see any LTP test parse a longer parameters.

> > Fixes: 180834982 ("kconfig: add funtion to parse /proc/cmdline")
> > Signed-off-by: Li Wang <liwang@redhat.com>
> > Cc: Petr Vorel <pvorel@suse.cz>
> > ---
> >  include/tst_kconfig.h | 2 +-
> >  lib/tst_kconfig.c     | 4 ++--
> >  2 files changed, 3 insertions(+), 3 deletions(-)

> > diff --git a/include/tst_kconfig.h b/include/tst_kconfig.h
> > index dcb370574..23f807409 100644
> > --- a/include/tst_kconfig.h
> > +++ b/include/tst_kconfig.h
> > @@ -87,7 +87,7 @@ int tst_kconfig_check(const char *const kconfigs[]);
> >   */
> >  struct tst_kcmdline_var {
> >         const char *key;
> > -       char value[128];
> > +       char value[256];
> >         bool found;
> >  };

> > diff --git a/lib/tst_kconfig.c b/lib/tst_kconfig.c
> > index e16ca1400..8eb1b803f 100644
> > --- a/lib/tst_kconfig.c
> > +++ b/lib/tst_kconfig.c
> > @@ -569,7 +569,7 @@ char tst_kconfig_get(const char *confname)

> >  void tst_kcmdline_parse(struct tst_kcmdline_var params[], size_t
> > params_len)
> >  {
> > -       char buf[128], line[512];
> > +       char buf[256], line[512];


> Petr, if you are still worried, feel free to enlarge them to char
> 'buf[512], line[2048];' during merge :).

Li, I'm ok with 256 :).
Reviewed-by: Petr Vorel <pvorel@suse.cz>

-				tst_res(TWARN, "Buffer overflowed while parsing /proc/cmdline");
+				tst_res(TINFO, "Buffer overflowed while parsing /proc/cmdline");

But I wonder if we should keep TWARN. Or at least add
+				tst_res(TINFO, "WARNING: Buffer overflowed while parsing /proc/cmdline");

BTW I also see Cyril's comment about adding TINFO | TWARN (or TINFO_WARN), where
he does not like neither of these two and even suggest to actually remove TWARN.
https://lore.kernel.org/ltp/ZldFa-3CXXbVKmVW@yuki/

> BTW, I don't want to allocate the size dynamically to make the code more
> complicated.

Fully agree.

Kind regards,
Petr

-- 
Mailing list info: https://lists.linux.it/listinfo/ltp