Re: [PATCH] test/argparse: fix out of bound memcpy

[Stephen Hemminger <stephen@networkplumber.org>]

On Fri, 27 Jun 2025 19:56:57 +0100
Bruce Richardson <bruce.richardson@intel.com> wrote:

> On Fri, Jun 27, 2025 at 09:22:35AM -0700, Stephen Hemminger wrote:
> > The rte_argparse API use variable length arrays for the args.
> > But the test was only putting space on stack for the argparse
> > part, not the args. This can lead to out of bounds writes.
> > 
> > The bug only gets detected if DPDK is compiled with LTO.
> > In function ‘test_argparse_copy’,
> >     inlined from ‘test_argparse_init_obj’ at ../app/test/test_argparse.c:108:2,
> >     inlined from ‘test_argparse_opt_callback_parse_int_of_no_val’ at ../app/test/test_argparse.c:490:8:
> > ../app/test/test_argparse.c:96:17: warning: ‘memcpy’ writing 56 bytes into a region of size 0 overflows the destination [-Wstringop-overflow=]
> >    96 |                 memcpy(&dst->args[i], &src->args[i], sizeof(src->args[i]));
> > 
> > Fixes: 6c5c6571601c ("argparse: verify argument config")
> > Cc: fengchengwen@huawei.com
> > Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>
> > ---  
> 
> It looks to me like this is a false positive. If it's not, then the whole
> method of declaring argparse arguments is broken, and the library is not
> really usable.
> 
> See below for what I see in gdb for a regular (non-LTO) debug build. Looks
> to me like the compiler is doing the right thing.
> 
> /Bruce

The problem is that the when structure is initialized its size gets boosted.
	https://www.gnu.org/software/c-intro-and-ref/manual/html_node/Flexible-Array-Fields.html
	GNU C allows static initialization of flexible array fields. 
	The effect is to “make the array long enough” for the initializer.

	struct f1 { int x; int y[]; } f1
		  = { 1, { 2, 3, 4 } };

It looks like a compiler bug that the extra size info doesn't get propogated
into the copy code.