Re: [PATCH libnetfilter_queue 2/2] pktbuff: add pktb_head_alloc(), pktb_setup() and pktb_head_size()

[Duncan Roe <duncan_roe@optusnet.com.au>]

On Sun, May 10, 2020 at 02:09:03AM +1000, Duncan Roe wrote:
> On Sat, May 09, 2020 at 11:11:41AM +0200, Pablo Neira Ayuso wrote:
[...]
> > diff --git a/src/extra/pktbuff.c b/src/extra/pktbuff.c
> > index 118ad898f63b..6acefbe72a9b 100644
> > --- a/src/extra/pktbuff.c
> > +++ b/src/extra/pktbuff.c
> > @@ -103,6 +103,26 @@ struct pkt_buff *pktb_alloc(int family, void *data, size_t len, size_t extra)
> >  	return pktb;
> >  }
> >
> > +EXPORT_SYMBOL
> > +struct pkt_buff *pktb_setup(struct pkt_buff *pktb, int family, uint8_t *buf,
> > +			    size_t len, size_t extra)
> > +{
> > +	pktb->data_len = len + extra;
>
> Are you proposing to be able to use extra space in the receive buffer?
> I think that is unsafe. mnl_cb_run() steps through that bufffer and needs a
> zero following the last message to know there are no more. At least, that's
> how it looks to me on stepping through with gdb.

That's wrong - sorry. mnl_cb_run() *does* step through the received buffer but
uses the byte count at the front of each message. When there are not enough
unprocessed bytes for another message header (usually there are zero) then it
returns. So there is room to enlarge the packet in the read buffer if it is in
the last (or only) netlink message in that buffer.

In tests I have never seen more than one netlink message returned in the receive
buffer. Even stopping the nfq program with gdb and then sending 10 or so UDP
messges, each call to mnl_socket_recvfrom() only returns a single netlink
message.

If this always happens, there is no need for memcpy() at all. And even if it
doesn't, one could programatically check whether the struct nlmsghdr passed by
queue_cb() is the last in the receive buffer and memcpy() if not.

Any comments?
>
[...]

Cheers ... Duncan.