TEST_NLATTR_OBJECT and TEST_NLATTR_ARRAY fault when sizeof(obj) - 1 > DEFAULT_STRLEN

[JingPiao Chen]

On Mon, Jul 03, 2017 at 10:23:49PM +0300, Dmitry V. Levin wrote:
> On Mon, Jul 03, 2017 at 08:05:42AM +0800, JingPiao Chen wrote:
> > On Mon, Jul 03, 2017 at 01:22:52AM +0300, Dmitry V. Levin wrote:
> > > On Sun, Jul 02, 2017 at 12:12:54PM +0800, JingPiao Chen wrote:
> > > > when sizeof(obj) - 1 > DEFAULT_STRLEN, we print all string,
> > > > but strace print "STR"..., so the test fault.
> > > > This appear when I test packet_diag_msg PACKET_DIAG_MCLIST.
> > > 
> > > Does this patch solve it?
> > 
> > Can solve it.
> > > 
> > > --- a/tests/test_nlattr.h
> > > +++ b/tests/test_nlattr.h
> > > @@ -99,20 +99,19 @@ print_nlattr(const unsigned int nla_len, const char *const nla_type)
> > >  			   init_msg_, print_msg_,			\
> > >  			   nla_type_, pattern_, obj_, ...)		\
> > >  	do {								\
> > > +		const size_t plen = sizeof(obj_) - 1 > DEFAULT_STRLEN	\
> > > +			? DEFAULT_STRLEN : sizeof(obj_) - 1;		\
> > >  		/* len < sizeof(obj_) */				\
> > >  		TEST_NLATTR_((fd_), (nlh0_), (hdrlen_),			\
> > >  			(init_msg_), (print_msg_),			\
> > >  			(nla_type_), #nla_type_,			\
> > > -			sizeof(obj_) - 1,				\
> > > -			(pattern_), sizeof(obj_) - 1,			\
> > > -			printf("\"%.*s\"",				\
> > > -			(int) sizeof(obj_) - 1,	(pattern_)));		\
> > > +			plen, (pattern_), plen,				\
> > > +			printf("\"%.*s\"", (int) plen, (pattern_)));	\
> > >  		/* short read of sizeof(obj_) */			\
> > >  		TEST_NLATTR_((fd_), (nlh0_), (hdrlen_),			\
> > >  			(init_msg_), (print_msg_),			\
> > >  			(nla_type_), #nla_type_,			\
> > > -			sizeof(obj_),					\
> > > -			(pattern_), sizeof(obj_) - 1,			\
> > > +			sizeof(obj_), (pattern_), plen,			\
> > 
> > I think here not need to replace with plen.
> > So can we named str_len instead of plen?
> 
> OK, changed to keep the same size.

I misunderstand your intention, if do not change here, memcpy in init_nlattr()
will access illegal memory. Now you adjust it properly, thank you.

> btw, I don't think printing unrecognized data as string is useful.
> After all, most likely it's not a string in the first place.
> Don't you think that printing it in hex (with QUOTE_FORCE_HEX flag set)
> would be more useful?

I agree, many structures first field is integer, print in hex it better.
I put that on my to-do list.

> 
> > >  			printf("%p",					\
> > >  			       RTA_DATA(NLMSG_ATTR(nlh, (hdrlen_)))));	\
> > >  		/* sizeof(obj_) */					\
> > > @@ -128,15 +127,15 @@ print_nlattr(const unsigned int nla_len, const char *const nla_type)
> > >  			  init_msg_, print_msg_,			\
> > >  			  nla_type_, pattern_, obj_, print_elem_)	\
> > >  	do {								\
> > > +		const size_t plen =					\
> > > +			sizeof((obj_)[0]) - 1 > DEFAULT_STRLEN		\
> > > +			? DEFAULT_STRLEN : sizeof((obj_)[0]) - 1;	\
> > >  		/* len < sizeof((obj_)[0]) */				\
> > >  		TEST_NLATTR_((fd_), (nlh0_), (hdrlen_),			\
> > >  			(init_msg_), (print_msg_),			\
> > >  			(nla_type_), #nla_type_,			\
> > > -			sizeof((obj_)[0]) - 1,				\
> > > -			(pattern_), sizeof((obj_)[0]) - 1,		\
> > > -			printf("\"%.*s\"",				\
> > > -			       (int) sizeof((obj_)[0]) - 1,		\
> > > -			       (pattern_)));				\
> > > +			plen, (pattern_), plen,				\
> > > +			printf("\"%.*s\"", (int) plen, (pattern_)));	\
> > >  		/* sizeof((obj_)[0]) < len < sizeof(obj_) */		\
> > >  		TEST_NLATTR_((fd_), (nlh0_), (hdrlen_),			\
> > >  			(init_msg_), (print_msg_),			\
> > > 
> > 
> > This patch require the caller define DEFAULT_STRLEN,
> > can we define DEFAULT_STRLEN in test_nlattr.h or tests.h?
> > Why we define DEFAULT_STRLEN dispersedly?
> 
> Moved DEFAULT_STRLEN to tests.h, thanks for the hint.

--
JingPiao Chen