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

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

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:
> > Add two new helper functions, as alternative to pktb_alloc().
> >
> > * pktb_setup() allows you to skip memcpy()'ing the payload from the
> >   netlink message.
> >
> > * pktb_head_size() returns the size of the pkt_buff opaque object.
> >
> > * pktb_head_alloc() allows you to allocate the pkt_buff in the heap.
> >
> > Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
> > ---
> >  include/libnetfilter_queue/pktbuff.h |  7 +++++++
> >  src/extra/pktbuff.c                  | 20 ++++++++++++++++++++
> >  2 files changed, 27 insertions(+)
> >
> > diff --git a/include/libnetfilter_queue/pktbuff.h b/include/libnetfilter_queue/pktbuff.h
> > index 42bc153ec337..a27582b02840 100644
> > --- a/include/libnetfilter_queue/pktbuff.h
> > +++ b/include/libnetfilter_queue/pktbuff.h
> > @@ -6,6 +6,13 @@ struct pkt_buff;
> >  struct pkt_buff *pktb_alloc(int family, void *data, size_t len, size_t extra);
> >  void pktb_free(struct pkt_buff *pktb);
> >
> > +#define NFQ_BUFFER_SIZE	(0xffff + (MNL_SOCKET_BUFFER_SIZE / 2)
> > +struct pkt_buff *pktb_setup(struct pkt_buff *pktb, int family, uint8_t *data,
> > +			    size_t len, size_t extra);
> > +size_t pktb_head_size(void);
> > +
> > +#define pktb_head_alloc()	(struct pkt_buff *)(malloc(pktb_head_size()))
> > +
> >  uint8_t *pktb_data(struct pkt_buff *pktb);
> >  uint32_t pktb_len(struct pkt_buff *pktb);
> >
> > 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.

There are "two buffers":

1) The buffer that you use to receive the netlink message. This buffer
   is parsed via mnl_cb_run().

2) The buffer that stores the pkt_buff structure.

pktb_setup() is called after mnl_cb_run(), once you have already
parsed the buffer that you have received from netlink. You might want
to pass the pointer to the data to mnl_cb_run().

If you would like to mangle the payload, then you can memcpy() the
attr[NFQA_PAYLOAD] and specify how many extra bytes (unused) are
available in the new buffer.

If you use attr[NFQA_PAYLOAD], then extra is zero.

This already allowing you to allocate the data in the stack as you
would like to do.