[PATCH] netlink: fix NLMSG_DONE never enter family specific decoder

[JingPiao Chen]

On Mon, Jun 26, 2017 at 03:13:42AM +0300, Dmitry V. Levin wrote:
> On Sat, Jun 24, 2017 at 01:35:18PM +0800, JingPiao Chen wrote:
> > * netlink.c (print_nlmsghdr): Remove condition
> > nlmsghdr->nlmsg_type < NLMSG_MIN_TYPE.
> > (decode_nlmsg_type): Add condition type >= NLMSG_MIN_TYPE.
> > (decode_nlmsg_flags): Add condition type < NLMSG_MIN_TYPE.
> > (decode_netlink_sock_diag): Add condition
> > nlmsghdr->nlmsg_type < NLMSG_MIN_TYPE.
> > ---
> >  netlink.c           | 13 ++++++++-----
> >  netlink_sock_diag.c |  3 +++
> >  2 files changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/netlink.c b/netlink.c
> > index 87b389c..d990fad 100644
> > --- a/netlink.c
> > +++ b/netlink.c
> > @@ -177,7 +177,7 @@ decode_nlmsg_type(const uint16_t type, const unsigned int family)
> >  	const struct xlat *xlat = netlink_types;
> >  	const char *dflt = "NLMSG_???";
> >  
> > -	if (family < ARRAY_SIZE(nlmsg_types)) {
> > +	if (type >= NLMSG_MIN_TYPE && family < ARRAY_SIZE(nlmsg_types)) {
> >  		if (nlmsg_types[family].decoder)
> >  			decoder = nlmsg_types[family].decoder;
> >  		if (nlmsg_types[family].xlat)
> > @@ -194,6 +194,11 @@ decode_nlmsg_flags(const uint16_t flags, const uint16_t type, const int family)
> >  {
> >  	const struct xlat *table = NULL;
> >  
> > +	if (type < NLMSG_MIN_TYPE) {
> > +		printflags(netlink_flags, flags, "NLM_F_???");
> > +		return;
> > +	}
> > +
> 
> There is another printflags invocation in this function already,
> can this duplication be avoided?

Can delete, goto statement, big if statement, change family, to tell the
truth, I do like these solution.
Finally, I replace it with goto statement.

> 
> >  	switch (family) {
> >  	case NETLINK_SOCK_DIAG:
> >  		table = netlink_get_flags;
> > @@ -246,10 +251,8 @@ print_nlmsghdr(struct tcb *tcp,
> >  
> >  	tprintf("{len=%u, type=", nlmsghdr->nlmsg_len);
> >  
> > -	const int hdr_family = (nlmsghdr->nlmsg_type < NLMSG_MIN_TYPE)
> > -			       ? NL_FAMILY_DEFAULT
> > -			       : (family != NL_FAMILY_DEFAULT
> > -				  ? family : get_fd_nl_family(tcp, fd));
> > +	const int hdr_family = family != NL_FAMILY_DEFAULT
> > +			       ? family : get_fd_nl_family(tcp, fd);
> 
> Is the family needed for any nlmsg_type?  What about NLMSG_ERROR?

Changed, do not delete previous condition,
add nlmsghdr->nlmsg_type != NLMSG_DONE.

	const int hdr_family = (nlmsghdr->nlmsg_type < NLMSG_MIN_TYPE
				&& nlmsghdr->nlmsg_type != NLMSG_DONE)
			       ? NL_FAMILY_DEFAULT
			       : (family != NL_FAMILY_DEFAULT
				  ? family : get_fd_nl_family(tcp, fd));

--
JingPiao Chen