Re: [PATCH] libnfnetlink

[Pablo Neira Ayuso <pablo@netfilter.org>]

Patrick McHardy wrote:
> Maik Hentsche wrote:
>> Hello Pablo, hello readers of the list,
>> I found another bug in libnfnetlink. The comment of nfnl_recv states, in
>> case of success 0 is returned. In fact at success the returnvalue of
>> recvfrom is returned, which is the number of received bytes
>> (libnfnetlink_recv_comment.patch). The second issue is a little more
>> serious. The comment states, in case of an error, errno is set when in
>> fact it is not. I appended a patch for two occurences, but I since I
>> don't know, in which case addrlen might be != sizeof(peer) and what
>> peer.nl_pid means (and therefore why it is a problem, if it's not 0)
>> two error cases without appropriate errno value still exist.
> 
> addrlen != sizeof(peer) should never happen. I can't think of anything
> better than EINVAL. nl_pid != 0 means the message originated in
> userspace and some other program is trying to feed us messages.
> We could handle this by just calling recvmsg again. But this is mainly
> because I can't think of a proper errno code for this either :)

what do you think about the following solution?

> 	if (len < sizeof(struct nlmsgerr)
> 	    || len < sizeof(struct nlmsghdr))

errno = EBADMSG;

> [...]
> 	if (addrlen != sizeof(peer))

errno = EINVAL;

> 		return -1;
> 
> 	if (peer.nl_pid != 0)

errno = ENOMSG;

> 		return -1;
> 
> 	nlh = (struct nlmsghdr *)buf;
> 	if (nlh->nlmsg_flags & MSG_TRUNC || status > len)

errno = ENOSPC;

> 		return -1;

-- 
The dawn of the fourth age of Linux firewalling is coming; a time of
great struggle and heroic deeds -- J.Kadlecsik got inspired by J.Morris