* [PATCH RFC libnetfilter_queue] src: Simplify struct pkt_buff @ 2020-01-10 4:09 Duncan Roe 2020-01-10 11:12 ` Pablo Neira Ayuso 0 siblings, 1 reply; 9+ messages in thread From: Duncan Roe @ 2020-01-10 4:09 UTC (permalink / raw) To: pablo; +Cc: netfilter-devel In struct pkt_buff: - We only ever needed any 2 of len, data and tail. This has caused bugs in the past, e.g. commit 8a4316f31. Delete len, and where the value of pktb->len was required, use new PKTB_LEN macro. - head and data always had the same value. head was in the minority, so replace with data where it was used. Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au> --- src/extra/ipv4.c | 2 +- src/extra/pktbuff.c | 21 +++++++-------------- src/internal.h | 3 +-- 3 files changed, 9 insertions(+), 17 deletions(-) diff --git a/src/extra/ipv4.c b/src/extra/ipv4.c index c03f23f..8e36861 100644 --- a/src/extra/ipv4.c +++ b/src/extra/ipv4.c @@ -70,7 +70,7 @@ int nfq_ip_set_transport_header(struct pkt_buff *pktb, struct iphdr *iph) int doff = iph->ihl * 4; /* Wrong offset to IPv4 payload. */ - if ((int)pktb->len - doff <= 0) + if ((int)PKTB_LEN - doff <= 0) return -1; pktb->transport_header = pktb->network_header + doff; diff --git a/src/extra/pktbuff.c b/src/extra/pktbuff.c index 37f6bc0..0b67029 100644 --- a/src/extra/pktbuff.c +++ b/src/extra/pktbuff.c @@ -62,12 +62,10 @@ struct pkt_buff *pktb_alloc(int family, void *data, size_t len, size_t extra) pkt_data = (uint8_t *)pktb + sizeof(struct pkt_buff); memcpy(pkt_data, data, len); - pktb->len = len; pktb->data_len = len + extra; - pktb->head = pkt_data; pktb->data = pkt_data; - pktb->tail = pktb->head + len; + pktb->tail = pktb->data + len; switch(family) { case AF_INET: @@ -121,7 +119,7 @@ uint8_t *pktb_data(struct pkt_buff *pktb) EXPORT_SYMBOL uint32_t pktb_len(struct pkt_buff *pktb) { - return pktb->len; + return PKTB_LEN; } /** @@ -169,7 +167,6 @@ EXPORT_SYMBOL void pktb_push(struct pkt_buff *pktb, unsigned int len) { pktb->data -= len; - pktb->len += len; } /** @@ -181,7 +178,6 @@ EXPORT_SYMBOL void pktb_pull(struct pkt_buff *pktb, unsigned int len) { pktb->data += len; - pktb->len -= len; } /** @@ -193,7 +189,6 @@ EXPORT_SYMBOL void pktb_put(struct pkt_buff *pktb, unsigned int len) { pktb->tail += len; - pktb->len += len; } /** @@ -204,8 +199,7 @@ void pktb_put(struct pkt_buff *pktb, unsigned int len) EXPORT_SYMBOL void pktb_trim(struct pkt_buff *pktb, unsigned int len) { - pktb->len = len; - pktb->tail = pktb->head + len; + pktb->tail = pktb->data + len; } /** @@ -224,7 +218,7 @@ void pktb_trim(struct pkt_buff *pktb, unsigned int len) EXPORT_SYMBOL unsigned int pktb_tailroom(struct pkt_buff *pktb) { - return pktb->data_len - pktb->len; + return pktb->data_len - PKTB_LEN; } /** @@ -277,17 +271,16 @@ static int pktb_expand_tail(struct pkt_buff *pktb, int extra) * reallocation. Instead, increase the size of the extra room in * the tail in pktb_alloc. */ - if (pktb->len + extra > pktb->data_len) + if (PKTB_LEN + extra > pktb->data_len) return 0; - pktb->len += extra; pktb->tail = pktb->tail + extra; return 1; } static int enlarge_pkt(struct pkt_buff *pktb, unsigned int extra) { - if (pktb->len + extra > 65535) + if (PKTB_LEN + extra > 65535) return 0; if (!pktb_expand_tail(pktb, extra - pktb_tailroom(pktb))) @@ -346,7 +339,7 @@ int pktb_mangle(struct pkt_buff *pktb, if (rep_len > match_len) pktb_put(pktb, rep_len - match_len); else - pktb_trim(pktb, pktb->len + rep_len - match_len); + pktb_trim(pktb, PKTB_LEN + rep_len - match_len); pktb->mangled = true; return 1; diff --git a/src/internal.h b/src/internal.h index d968325..95ac03e 100644 --- a/src/internal.h +++ b/src/internal.h @@ -9,6 +9,7 @@ #else # define EXPORT_SYMBOL #endif +#define PKTB_LEN (pktb->tail - pktb->data) struct iphdr; struct ip6_hdr; @@ -23,11 +24,9 @@ struct pkt_buff { uint8_t *network_header; uint8_t *transport_header; - uint8_t *head; uint8_t *data; uint8_t *tail; - uint32_t len; uint32_t data_len; bool mangled; -- 2.14.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH RFC libnetfilter_queue] src: Simplify struct pkt_buff 2020-01-10 4:09 [PATCH RFC libnetfilter_queue] src: Simplify struct pkt_buff Duncan Roe @ 2020-01-10 11:12 ` Pablo Neira Ayuso 2020-01-17 11:32 ` [PATCH libnetfilter_queue] src: Simplify struct pkt_buff: remove head Duncan Roe 0 siblings, 1 reply; 9+ messages in thread From: Pablo Neira Ayuso @ 2020-01-10 11:12 UTC (permalink / raw) To: Duncan Roe; +Cc: netfilter-devel On Fri, Jan 10, 2020 at 03:09:25PM +1100, Duncan Roe wrote: > In struct pkt_buff: > - We only ever needed any 2 of len, data and tail. You can remove ->tail. > This has caused bugs in the past, e.g. commit 8a4316f31. > Delete len, and where the value of pktb->len was required, > use new PKTB_LEN macro. I would leave pktb->len in please, it keeps things simple. Otherwise, I would suggest to remove pktb->data_len, keep pktb->len in place and reallocate the data buffer only in case of mangling. > - head and data always had the same value. > head was in the minority, so replace with data where it was used. This item above also looks fine. Probably you can split this in several patches, one for each item. Thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH libnetfilter_queue] src: Simplify struct pkt_buff: remove head 2020-01-10 11:12 ` Pablo Neira Ayuso @ 2020-01-17 11:32 ` Duncan Roe 2020-01-17 14:09 ` [PATCH libnetfilter_queue] src: Simplify struct pkt_buff: remove tail Duncan Roe 2020-01-18 20:40 ` [PATCH libnetfilter_queue] src: Simplify struct pkt_buff: remove head Pablo Neira Ayuso 0 siblings, 2 replies; 9+ messages in thread From: Duncan Roe @ 2020-01-17 11:32 UTC (permalink / raw) To: pablo; +Cc: netfilter-devel head and data always had the same value. head was in the minority, so replace with data where it was used. Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au> --- src/extra/pktbuff.c | 5 ++--- src/internal.h | 1 - 2 files changed, 2 insertions(+), 4 deletions(-) diff --git a/src/extra/pktbuff.c b/src/extra/pktbuff.c index f013cfe..c95384f 100644 --- a/src/extra/pktbuff.c +++ b/src/extra/pktbuff.c @@ -66,9 +66,8 @@ struct pkt_buff *pktb_alloc(int family, void *data, size_t len, size_t extra) pktb->len = len; pktb->data_len = len + extra; - pktb->head = pkt_data; pktb->data = pkt_data; - pktb->tail = pktb->head + len; + pktb->tail = pktb->data + len; switch(family) { case AF_INET: @@ -204,7 +203,7 @@ EXPORT_SYMBOL void pktb_trim(struct pkt_buff *pktb, unsigned int len) { pktb->len = len; - pktb->tail = pktb->head + len; + pktb->tail = pktb->data + len; } /** diff --git a/src/internal.h b/src/internal.h index d968325..0cfa425 100644 --- a/src/internal.h +++ b/src/internal.h @@ -23,7 +23,6 @@ struct pkt_buff { uint8_t *network_header; uint8_t *transport_header; - uint8_t *head; uint8_t *data; uint8_t *tail; -- 2.14.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* [PATCH libnetfilter_queue] src: Simplify struct pkt_buff: remove tail 2020-01-17 11:32 ` [PATCH libnetfilter_queue] src: Simplify struct pkt_buff: remove head Duncan Roe @ 2020-01-17 14:09 ` Duncan Roe 2020-01-18 20:43 ` Pablo Neira Ayuso 2020-01-18 20:40 ` [PATCH libnetfilter_queue] src: Simplify struct pkt_buff: remove head Pablo Neira Ayuso 1 sibling, 1 reply; 9+ messages in thread From: Duncan Roe @ 2020-01-17 14:09 UTC (permalink / raw) To: pablo; +Cc: netfilter-devel In struct pkt_buff, we only ever needed any 2 of len, data and tail. This has caused bugs in the past, e.g. commit 8a4316f31. Delete tail, and where the value of pktb->tail was required, use new PKTB_TAIL macro. Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au> --- src/extra/ipv4.c | 4 ++-- src/extra/ipv6.c | 8 ++++---- src/extra/pktbuff.c | 6 +----- src/extra/tcp.c | 6 +++--- src/extra/udp.c | 6 +++--- src/internal.h | 2 +- 6 files changed, 14 insertions(+), 18 deletions(-) diff --git a/src/extra/ipv4.c b/src/extra/ipv4.c index caafd37..bd5da22 100644 --- a/src/extra/ipv4.c +++ b/src/extra/ipv4.c @@ -40,7 +40,7 @@ EXPORT_SYMBOL struct iphdr *nfq_ip_get_hdr(struct pkt_buff *pktb) { struct iphdr *iph; - unsigned int pktlen = pktb->tail - pktb->network_header; + unsigned int pktlen = PKTB_TAIL - pktb->network_header; /* Not enough room for IPv4 header. */ if (pktlen < sizeof(struct iphdr)) @@ -135,7 +135,7 @@ int nfq_ip_mangle(struct pkt_buff *pktb, unsigned int dataoff, return 0; /* fix IP hdr checksum information */ - iph->tot_len = htons(pktb->tail - pktb->network_header); + iph->tot_len = htons(PKTB_TAIL - pktb->network_header); nfq_ip_set_checksum(iph); return 1; diff --git a/src/extra/ipv6.c b/src/extra/ipv6.c index 6e8820c..b6542f9 100644 --- a/src/extra/ipv6.c +++ b/src/extra/ipv6.c @@ -36,7 +36,7 @@ EXPORT_SYMBOL struct ip6_hdr *nfq_ip6_get_hdr(struct pkt_buff *pktb) { struct ip6_hdr *ip6h; - unsigned int pktlen = pktb->tail - pktb->network_header; + unsigned int pktlen = PKTB_TAIL - pktb->network_header; /* Not enough room for IPv6 header. */ if (pktlen < sizeof(struct ip6_hdr)) @@ -77,7 +77,7 @@ int nfq_ip6_set_transport_header(struct pkt_buff *pktb, struct ip6_hdr *ip6h, break; } /* No room for extension, bad packet. */ - if (pktb->tail - cur < sizeof(struct ip6_ext)) { + if (PKTB_TAIL - cur < sizeof(struct ip6_ext)) { cur = NULL; break; } @@ -87,7 +87,7 @@ int nfq_ip6_set_transport_header(struct pkt_buff *pktb, struct ip6_hdr *ip6h, uint16_t *frag_off; /* No room for full fragment header, bad packet. */ - if (pktb->tail - cur < sizeof(struct ip6_frag)) { + if (PKTB_TAIL - cur < sizeof(struct ip6_frag)) { cur = NULL; break; } @@ -140,7 +140,7 @@ int nfq_ip6_mangle(struct pkt_buff *pktb, unsigned int dataoff, /* Fix IPv6 hdr length information */ ip6h->ip6_plen = - htons(pktb->tail - pktb->network_header - sizeof *ip6h); + htons(PKTB_TAIL - pktb->network_header - sizeof *ip6h); return 1; } diff --git a/src/extra/pktbuff.c b/src/extra/pktbuff.c index c95384f..f97750f 100644 --- a/src/extra/pktbuff.c +++ b/src/extra/pktbuff.c @@ -67,7 +67,6 @@ struct pkt_buff *pktb_alloc(int family, void *data, size_t len, size_t extra) pktb->data_len = len + extra; pktb->data = pkt_data; - pktb->tail = pktb->data + len; switch(family) { case AF_INET: @@ -190,7 +189,6 @@ void pktb_pull(struct pkt_buff *pktb, unsigned int len) EXPORT_SYMBOL void pktb_put(struct pkt_buff *pktb, unsigned int len) { - pktb->tail += len; pktb->len += len; } @@ -203,7 +201,6 @@ EXPORT_SYMBOL void pktb_trim(struct pkt_buff *pktb, unsigned int len) { pktb->len = len; - pktb->tail = pktb->data + len; } /** @@ -279,7 +276,6 @@ static int pktb_expand_tail(struct pkt_buff *pktb, int extra) return 0; pktb->len += extra; - pktb->tail = pktb->tail + extra; return 1; } @@ -334,7 +330,7 @@ int pktb_mangle(struct pkt_buff *pktb, /* move post-replacement */ memmove(data + match_offset + rep_len, data + match_offset + match_len, - pktb->tail - (pktb->network_header + dataoff + + PKTB_TAIL - (pktb->network_header + dataoff + match_offset + match_len)); /* insert data from buffer */ diff --git a/src/extra/tcp.c b/src/extra/tcp.c index cca20e7..eb43067 100644 --- a/src/extra/tcp.c +++ b/src/extra/tcp.c @@ -46,7 +46,7 @@ struct tcphdr *nfq_tcp_get_hdr(struct pkt_buff *pktb) return NULL; /* No room for the TCP header. */ - if (pktb->tail - pktb->transport_header < sizeof(struct tcphdr)) + if (PKTB_TAIL - pktb->transport_header < sizeof(struct tcphdr)) return NULL; return (struct tcphdr *)pktb->transport_header; @@ -68,7 +68,7 @@ void *nfq_tcp_get_payload(struct tcphdr *tcph, struct pkt_buff *pktb) return NULL; /* malformed TCP data offset. */ - if (pktb->transport_header + len > pktb->tail) + if (pktb->transport_header + len > PKTB_TAIL) return NULL; return pktb->transport_header + len; @@ -83,7 +83,7 @@ void *nfq_tcp_get_payload(struct tcphdr *tcph, struct pkt_buff *pktb) EXPORT_SYMBOL unsigned int nfq_tcp_get_payload_len(struct tcphdr *tcph, struct pkt_buff *pktb) { - return pktb->tail - pktb->transport_header - (tcph->doff * 4); + return PKTB_TAIL - pktb->transport_header - (tcph->doff * 4); } /** diff --git a/src/extra/udp.c b/src/extra/udp.c index dc476d4..4b7e3cc 100644 --- a/src/extra/udp.c +++ b/src/extra/udp.c @@ -46,7 +46,7 @@ struct udphdr *nfq_udp_get_hdr(struct pkt_buff *pktb) return NULL; /* No room for the UDP header. */ - if (pktb->tail - pktb->transport_header < sizeof(struct udphdr)) + if (PKTB_TAIL - pktb->transport_header < sizeof(struct udphdr)) return NULL; return (struct udphdr *)pktb->transport_header; @@ -68,7 +68,7 @@ void *nfq_udp_get_payload(struct udphdr *udph, struct pkt_buff *pktb) return NULL; /* malformed UDP packet. */ - if (pktb->transport_header + len > pktb->tail) + if (pktb->transport_header + len > PKTB_TAIL) return NULL; return pktb->transport_header + sizeof(struct udphdr); @@ -83,7 +83,7 @@ void *nfq_udp_get_payload(struct udphdr *udph, struct pkt_buff *pktb) EXPORT_SYMBOL unsigned int nfq_udp_get_payload_len(struct udphdr *udph, struct pkt_buff *pktb) { - return pktb->tail - pktb->transport_header - sizeof(struct udphdr); + return PKTB_TAIL - pktb->transport_header - sizeof(struct udphdr); } /** diff --git a/src/internal.h b/src/internal.h index 0cfa425..dafb33a 100644 --- a/src/internal.h +++ b/src/internal.h @@ -9,6 +9,7 @@ #else # define EXPORT_SYMBOL #endif +#define PKTB_TAIL (pktb->data + pktb->len) struct iphdr; struct ip6_hdr; @@ -24,7 +25,6 @@ struct pkt_buff { uint8_t *transport_header; uint8_t *data; - uint8_t *tail; uint32_t len; uint32_t data_len; -- 2.14.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH libnetfilter_queue] src: Simplify struct pkt_buff: remove tail 2020-01-17 14:09 ` [PATCH libnetfilter_queue] src: Simplify struct pkt_buff: remove tail Duncan Roe @ 2020-01-18 20:43 ` Pablo Neira Ayuso 2020-01-26 4:02 ` [PATCH libnetfilter_queue v2 0/1] " Duncan Roe 2020-01-26 4:02 ` [PATCH libnetfilter_queue v2 1/1] " Duncan Roe 0 siblings, 2 replies; 9+ messages in thread From: Pablo Neira Ayuso @ 2020-01-18 20:43 UTC (permalink / raw) To: Duncan Roe; +Cc: netfilter-devel On Sat, Jan 18, 2020 at 01:09:55AM +1100, Duncan Roe wrote: [...] > diff --git a/src/internal.h b/src/internal.h > index 0cfa425..dafb33a 100644 > --- a/src/internal.h > +++ b/src/internal.h > @@ -9,6 +9,7 @@ > #else > # define EXPORT_SYMBOL > #endif > +#define PKTB_TAIL (pktb->data + pktb->len) Instead of a macro, I'd suggest you add (something like): static inline uint8_t *pktb_tail(struct pktbuff *pktb) { return pktb->data + pktb->len; } Thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH libnetfilter_queue v2 0/1] Simplify struct pkt_buff: remove tail 2020-01-18 20:43 ` Pablo Neira Ayuso @ 2020-01-26 4:02 ` Duncan Roe 2020-01-26 4:02 ` [PATCH libnetfilter_queue v2 1/1] " Duncan Roe 1 sibling, 0 replies; 9+ messages in thread From: Duncan Roe @ 2020-01-26 4:02 UTC (permalink / raw) To: pablo; +Cc: netfilter-devel Hi Pablo, This patch uses a static inline function as you requested. I thought the macro would result in fewer instructions, but haven't checked. Anyway you have both versions now. Cheers ... Duncan. Duncan Roe (1): Simplify struct pkt_buff: remove tail src/extra/ipv4.c | 4 ++-- src/extra/ipv6.c | 8 ++++---- src/extra/pktbuff.c | 6 +----- src/extra/tcp.c | 6 +++--- src/extra/udp.c | 6 +++--- src/internal.h | 5 ++++- 6 files changed, 17 insertions(+), 18 deletions(-) -- 2.14.5 ^ permalink raw reply [flat|nested] 9+ messages in thread
* [PATCH libnetfilter_queue v2 1/1] Simplify struct pkt_buff: remove tail 2020-01-18 20:43 ` Pablo Neira Ayuso 2020-01-26 4:02 ` [PATCH libnetfilter_queue v2 0/1] " Duncan Roe @ 2020-01-26 4:02 ` Duncan Roe 2020-01-29 17:39 ` Pablo Neira Ayuso 1 sibling, 1 reply; 9+ messages in thread From: Duncan Roe @ 2020-01-26 4:02 UTC (permalink / raw) To: pablo; +Cc: netfilter-devel In struct pkt_buff, we only ever needed any 2 of len, data and tail. This has caused bugs in the past, e.g. commit 8a4316f31. Delete tail, and where the value of pktb->tail was required, use new pktb_tail() function. Signed-off-by: Duncan Roe <duncan_roe@optusnet.com.au> --- src/extra/ipv4.c | 4 ++-- src/extra/ipv6.c | 8 ++++---- src/extra/pktbuff.c | 6 +----- src/extra/tcp.c | 6 +++--- src/extra/udp.c | 6 +++--- src/internal.h | 5 ++++- 6 files changed, 17 insertions(+), 18 deletions(-) diff --git a/src/extra/ipv4.c b/src/extra/ipv4.c index caafd37..797bab1 100644 --- a/src/extra/ipv4.c +++ b/src/extra/ipv4.c @@ -40,7 +40,7 @@ EXPORT_SYMBOL struct iphdr *nfq_ip_get_hdr(struct pkt_buff *pktb) { struct iphdr *iph; - unsigned int pktlen = pktb->tail - pktb->network_header; + unsigned int pktlen = pktb_tail(pktb) - pktb->network_header; /* Not enough room for IPv4 header. */ if (pktlen < sizeof(struct iphdr)) @@ -135,7 +135,7 @@ int nfq_ip_mangle(struct pkt_buff *pktb, unsigned int dataoff, return 0; /* fix IP hdr checksum information */ - iph->tot_len = htons(pktb->tail - pktb->network_header); + iph->tot_len = htons(pktb_tail(pktb) - pktb->network_header); nfq_ip_set_checksum(iph); return 1; diff --git a/src/extra/ipv6.c b/src/extra/ipv6.c index 6e8820c..42c5e25 100644 --- a/src/extra/ipv6.c +++ b/src/extra/ipv6.c @@ -36,7 +36,7 @@ EXPORT_SYMBOL struct ip6_hdr *nfq_ip6_get_hdr(struct pkt_buff *pktb) { struct ip6_hdr *ip6h; - unsigned int pktlen = pktb->tail - pktb->network_header; + unsigned int pktlen = pktb_tail(pktb) - pktb->network_header; /* Not enough room for IPv6 header. */ if (pktlen < sizeof(struct ip6_hdr)) @@ -77,7 +77,7 @@ int nfq_ip6_set_transport_header(struct pkt_buff *pktb, struct ip6_hdr *ip6h, break; } /* No room for extension, bad packet. */ - if (pktb->tail - cur < sizeof(struct ip6_ext)) { + if (pktb_tail(pktb) - cur < sizeof(struct ip6_ext)) { cur = NULL; break; } @@ -87,7 +87,7 @@ int nfq_ip6_set_transport_header(struct pkt_buff *pktb, struct ip6_hdr *ip6h, uint16_t *frag_off; /* No room for full fragment header, bad packet. */ - if (pktb->tail - cur < sizeof(struct ip6_frag)) { + if (pktb_tail(pktb) - cur < sizeof(struct ip6_frag)) { cur = NULL; break; } @@ -140,7 +140,7 @@ int nfq_ip6_mangle(struct pkt_buff *pktb, unsigned int dataoff, /* Fix IPv6 hdr length information */ ip6h->ip6_plen = - htons(pktb->tail - pktb->network_header - sizeof *ip6h); + htons(pktb_tail(pktb) - pktb->network_header - sizeof *ip6h); return 1; } diff --git a/src/extra/pktbuff.c b/src/extra/pktbuff.c index c95384f..6dd0ca9 100644 --- a/src/extra/pktbuff.c +++ b/src/extra/pktbuff.c @@ -67,7 +67,6 @@ struct pkt_buff *pktb_alloc(int family, void *data, size_t len, size_t extra) pktb->data_len = len + extra; pktb->data = pkt_data; - pktb->tail = pktb->data + len; switch(family) { case AF_INET: @@ -190,7 +189,6 @@ void pktb_pull(struct pkt_buff *pktb, unsigned int len) EXPORT_SYMBOL void pktb_put(struct pkt_buff *pktb, unsigned int len) { - pktb->tail += len; pktb->len += len; } @@ -203,7 +201,6 @@ EXPORT_SYMBOL void pktb_trim(struct pkt_buff *pktb, unsigned int len) { pktb->len = len; - pktb->tail = pktb->data + len; } /** @@ -279,7 +276,6 @@ static int pktb_expand_tail(struct pkt_buff *pktb, int extra) return 0; pktb->len += extra; - pktb->tail = pktb->tail + extra; return 1; } @@ -334,7 +330,7 @@ int pktb_mangle(struct pkt_buff *pktb, /* move post-replacement */ memmove(data + match_offset + rep_len, data + match_offset + match_len, - pktb->tail - (pktb->network_header + dataoff + + pktb_tail(pktb) - (pktb->network_header + dataoff + match_offset + match_len)); /* insert data from buffer */ diff --git a/src/extra/tcp.c b/src/extra/tcp.c index cca20e7..933c6ee 100644 --- a/src/extra/tcp.c +++ b/src/extra/tcp.c @@ -46,7 +46,7 @@ struct tcphdr *nfq_tcp_get_hdr(struct pkt_buff *pktb) return NULL; /* No room for the TCP header. */ - if (pktb->tail - pktb->transport_header < sizeof(struct tcphdr)) + if (pktb_tail(pktb) - pktb->transport_header < sizeof(struct tcphdr)) return NULL; return (struct tcphdr *)pktb->transport_header; @@ -68,7 +68,7 @@ void *nfq_tcp_get_payload(struct tcphdr *tcph, struct pkt_buff *pktb) return NULL; /* malformed TCP data offset. */ - if (pktb->transport_header + len > pktb->tail) + if (pktb->transport_header + len > pktb_tail(pktb)) return NULL; return pktb->transport_header + len; @@ -83,7 +83,7 @@ void *nfq_tcp_get_payload(struct tcphdr *tcph, struct pkt_buff *pktb) EXPORT_SYMBOL unsigned int nfq_tcp_get_payload_len(struct tcphdr *tcph, struct pkt_buff *pktb) { - return pktb->tail - pktb->transport_header - (tcph->doff * 4); + return pktb_tail(pktb) - pktb->transport_header - (tcph->doff * 4); } /** diff --git a/src/extra/udp.c b/src/extra/udp.c index dc476d4..f232127 100644 --- a/src/extra/udp.c +++ b/src/extra/udp.c @@ -46,7 +46,7 @@ struct udphdr *nfq_udp_get_hdr(struct pkt_buff *pktb) return NULL; /* No room for the UDP header. */ - if (pktb->tail - pktb->transport_header < sizeof(struct udphdr)) + if (pktb_tail(pktb) - pktb->transport_header < sizeof(struct udphdr)) return NULL; return (struct udphdr *)pktb->transport_header; @@ -68,7 +68,7 @@ void *nfq_udp_get_payload(struct udphdr *udph, struct pkt_buff *pktb) return NULL; /* malformed UDP packet. */ - if (pktb->transport_header + len > pktb->tail) + if (pktb->transport_header + len > pktb_tail(pktb)) return NULL; return pktb->transport_header + sizeof(struct udphdr); @@ -83,7 +83,7 @@ void *nfq_udp_get_payload(struct udphdr *udph, struct pkt_buff *pktb) EXPORT_SYMBOL unsigned int nfq_udp_get_payload_len(struct udphdr *udph, struct pkt_buff *pktb) { - return pktb->tail - pktb->transport_header - sizeof(struct udphdr); + return pktb_tail(pktb) - pktb->transport_header - sizeof(struct udphdr); } /** diff --git a/src/internal.h b/src/internal.h index 0cfa425..ae849d6 100644 --- a/src/internal.h +++ b/src/internal.h @@ -24,7 +24,6 @@ struct pkt_buff { uint8_t *transport_header; uint8_t *data; - uint8_t *tail; uint32_t len; uint32_t data_len; @@ -32,4 +31,8 @@ struct pkt_buff { bool mangled; }; +static inline uint8_t *pktb_tail(struct pkt_buff *pktb) +{ + return pktb->data + pktb->len; +} #endif -- 2.14.5 ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH libnetfilter_queue v2 1/1] Simplify struct pkt_buff: remove tail 2020-01-26 4:02 ` [PATCH libnetfilter_queue v2 1/1] " Duncan Roe @ 2020-01-29 17:39 ` Pablo Neira Ayuso 0 siblings, 0 replies; 9+ messages in thread From: Pablo Neira Ayuso @ 2020-01-29 17:39 UTC (permalink / raw) To: Duncan Roe; +Cc: netfilter-devel On Sun, Jan 26, 2020 at 03:02:02PM +1100, Duncan Roe wrote: > In struct pkt_buff, we only ever needed any 2 of len, data and tail. > This has caused bugs in the past, e.g. commit 8a4316f31. > Delete tail, and where the value of pktb->tail was required, > use new pktb_tail() function. Applied, thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH libnetfilter_queue] src: Simplify struct pkt_buff: remove head 2020-01-17 11:32 ` [PATCH libnetfilter_queue] src: Simplify struct pkt_buff: remove head Duncan Roe 2020-01-17 14:09 ` [PATCH libnetfilter_queue] src: Simplify struct pkt_buff: remove tail Duncan Roe @ 2020-01-18 20:40 ` Pablo Neira Ayuso 1 sibling, 0 replies; 9+ messages in thread From: Pablo Neira Ayuso @ 2020-01-18 20:40 UTC (permalink / raw) To: Duncan Roe; +Cc: netfilter-devel On Fri, Jan 17, 2020 at 10:32:03PM +1100, Duncan Roe wrote: > head and data always had the same value. > head was in the minority, so replace with data where it was used. Applied, thanks. ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2020-01-29 17:39 UTC | newest] Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2020-01-10 4:09 [PATCH RFC libnetfilter_queue] src: Simplify struct pkt_buff Duncan Roe 2020-01-10 11:12 ` Pablo Neira Ayuso 2020-01-17 11:32 ` [PATCH libnetfilter_queue] src: Simplify struct pkt_buff: remove head Duncan Roe 2020-01-17 14:09 ` [PATCH libnetfilter_queue] src: Simplify struct pkt_buff: remove tail Duncan Roe 2020-01-18 20:43 ` Pablo Neira Ayuso 2020-01-26 4:02 ` [PATCH libnetfilter_queue v2 0/1] " Duncan Roe 2020-01-26 4:02 ` [PATCH libnetfilter_queue v2 1/1] " Duncan Roe 2020-01-29 17:39 ` Pablo Neira Ayuso 2020-01-18 20:40 ` [PATCH libnetfilter_queue] src: Simplify struct pkt_buff: remove head Pablo Neira Ayuso
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).