netlink: NLMSG_DONE never enter specific family decoder

[JingPiao Chen]

On Sat, Jun 24, 2017 at 04:36:34AM +0300, Dmitry V. Levin wrote:
> On Sat, Jun 24, 2017 at 09:06:06AM +0800, JingPiao Chen wrote:
> > static int
> > print_nlmsghdr(struct tcb *tcp,
> >       const int fd,
> >       int family,
> >       const struct nlmsghdr *const nlmsghdr)
> > {
> > ...
> > const int hdr_family = (nlmsghdr->nlmsg_type < NLMSG_MIN_TYPE)
> >       ? NL_FAMILY_DEFAULT
> >       : (family != NL_FAMILY_DEFAULT
> >  ? family : get_fd_nl_family(tcp, fd));
> 
> Feel free to change this if NLMSG_DONE needs a family specific decoding.

I have send a patch. I've also thought about rewriting NLMSG_DONE,
because only three family send useful data: drivers/connector/connector.c,
drivers/infiniband/core/iwpm_util.c and net/netfilter/nfnetlink_log.c.
In the end, I think it's good enough, but I want to ask your opinion.

$ git grep "NLMSG_DONE"

drivers/connector/connector.c:	nlh = nlmsg_put(skb, 0, msg->seq, NLMSG_DONE, size, 0);
drivers/infiniband/core/iwpm_util.c:	nlh->nlmsg_type = NLMSG_DONE;

These two family I do not understand what they send.

net/netfilter/nfnetlink_log.c:
static void
__nfulnl_send(struct nfulnl_instance *inst)
{
	if (inst->qlen > 1) {
		struct nlmsghdr *nlh = nlmsg_put(inst->skb, 0, 0,
						 NLMSG_DONE,
						 sizeof(struct nfgenmsg),
						 0);
		if (WARN_ONCE(!nlh, "bad nlskb size: %u, tailroom %d\n",
			      inst->skb->len, skb_tailroom(inst->skb))) {
			kfree_skb(inst->skb);
			goto out;
		}
	}
	nfnetlink_unicast(inst->skb, inst->net, inst->peer_portid,
			  MSG_DONTWAIT);
out:
	inst->qlen = 0;
	inst->skb = NULL;
}

It allocate struct nfgenmsg, but do not initialize it.

Following family data length is zero. Default decoder can handle them.

drivers/net/team/team.c: nlh = nlmsg_put(skb, portid, seq, NLMSG_DONE, 0, flags | NLM_F_MULTI);

drivers/scsi/scsi_transport_iscsi.c:	int t = done ? NLMSG_DONE : type;
static void
iscsi_if_rx(struct sk_buff *skb)
{
	...
			err = iscsi_if_send_reply(group, nlh->nlmsg_seq,
				nlh->nlmsg_type, 0, 0, ev, sizeof(*ev));
	...
}

kernel/audit.c:	int		t     = done  ? NLMSG_DONE  : type;
kernel/auditfilter.c:
static void audit_list_rules(int seq, struct sk_buff_head *q)
{
	...
	skb = audit_make_reply(seq, AUDIT_LIST_RULES, 1, 1, NULL, 0);
	...
}

net/core/devlink.c:			NLMSG_DONE, 0, flags | NLM_F_MULTI);
net/core/devlink.c:			NLMSG_DONE, 0, flags | NLM_F_MULTI);
net/core/devlink.c:			NLMSG_DONE, 0, flags | NLM_F_MULTI);

--
JingPiao Chen