* [DCCP]: Fix skb->cb conflicts with IP
@ 2008-04-04 12:13 Patrick McHardy
2008-04-04 13:25 ` Gerrit Renker
2008-04-04 13:26 ` Arnaldo Carvalho de Melo
0 siblings, 2 replies; 11+ messages in thread
From: Patrick McHardy @ 2008-04-04 12:13 UTC (permalink / raw)
To: acme; +Cc: dccp, Linux Netdev List
[-- Attachment #1: Type: text/plain, Size: 0 bytes --]
[-- Attachment #2: x --]
[-- Type: text/plain, Size: 1773 bytes --]
commit eced67957ee99f7b5fafdc73a58bcd037a1789b2
Author: Patrick McHardy <kaber@trash.net>
Date: Fri Apr 4 14:10:23 2008 +0200
[DCCP]: Fix skb->cb conflicts with IP
dev_queue_xmit() and the other IP output functions expect to get a skb
with clear or properly initialized skb->cb. Unlike TCP and UDP, the
dccp_skb_cb doesn't contain a struct inet_skb_parm at the beginning,
so the DCCP-specific data is interpreted by the IP output functions.
This can cause false negatives for the conditional POST_ROUTING hook
invocation, making the packet bypass the hook.
Add a inet_skb_parm/inet6_skb_parm union to the beginning of
dccp_skb_cb to avoid clashes. Also add a BUILD_BUG_ON to make
sure it fits in the cb.
Signed-off-by: Patrick McHardy <kaber@trash.net>
diff --git a/net/dccp/dccp.h b/net/dccp/dccp.h
index fe7726b..f44d492 100644
--- a/net/dccp/dccp.h
+++ b/net/dccp/dccp.h
@@ -325,6 +325,12 @@ static inline int dccp_bad_service_code(const struct sock *sk,
* This is used for transmission as well as for reception.
*/
struct dccp_skb_cb {
+ union {
+ struct inet_skb_parm h4;
+#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE)
+ struct inet6_skb_parm h6;
+#endif
+ } header;
__u8 dccpd_type:4;
__u8 dccpd_ccval:4;
__u8 dccpd_reset_code,
diff --git a/net/dccp/proto.c b/net/dccp/proto.c
index e3f5d37..c91d3c1 100644
--- a/net/dccp/proto.c
+++ b/net/dccp/proto.c
@@ -1057,6 +1057,9 @@ static int __init dccp_init(void)
int ehash_order, bhash_order, i;
int rc = -ENOBUFS;
+ BUILD_BUG_ON(sizeof(struct dccp_skb_cb) >
+ FIELD_SIZEOF(struct sk_buff, cb));
+
dccp_hashinfo.bind_bucket_cachep =
kmem_cache_create("dccp_bind_bucket",
sizeof(struct inet_bind_bucket), 0,
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [DCCP]: Fix skb->cb conflicts with IP 2008-04-04 12:13 [DCCP]: Fix skb->cb conflicts with IP Patrick McHardy @ 2008-04-04 13:25 ` Gerrit Renker 2008-04-04 13:40 ` Patrick McHardy 2008-04-04 13:47 ` Arnaldo Carvalho de Melo 2008-04-04 13:26 ` Arnaldo Carvalho de Melo 1 sibling, 2 replies; 11+ messages in thread From: Gerrit Renker @ 2008-04-04 13:25 UTC (permalink / raw) To: Patrick McHardy; +Cc: acme, dccp, Linux Netdev List Arnaldo, just a thought - I recall that there used to be a bug related to this, which required to insert the following before sending an skb: memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)) This was about 1+1/2 .. 2 years ago and lead to crashes when the memset was removed. Maybe with this solution the memsets are then no longer necessary? The reference is * output.c:dccp_transmit_skb() * ipv4.c:dccp_v4_send_response() Gerrit | commit eced67957ee99f7b5fafdc73a58bcd037a1789b2 | Author: Patrick McHardy <kaber@trash.net> | Date: Fri Apr 4 14:10:23 2008 +0200 | | [DCCP]: Fix skb->cb conflicts with IP | | dev_queue_xmit() and the other IP output functions expect to get a skb | with clear or properly initialized skb->cb. Unlike TCP and UDP, the | dccp_skb_cb doesn't contain a struct inet_skb_parm at the beginning, | so the DCCP-specific data is interpreted by the IP output functions. | This can cause false negatives for the conditional POST_ROUTING hook | invocation, making the packet bypass the hook. | | Add a inet_skb_parm/inet6_skb_parm union to the beginning of | dccp_skb_cb to avoid clashes. Also add a BUILD_BUG_ON to make | sure it fits in the cb. | | Signed-off-by: Patrick McHardy <kaber@trash.net> | | diff --git a/net/dccp/dccp.h b/net/dccp/dccp.h | index fe7726b..f44d492 100644 | --- a/net/dccp/dccp.h | +++ b/net/dccp/dccp.h | @@ -325,6 +325,12 @@ static inline int dccp_bad_service_code(const struct sock *sk, | * This is used for transmission as well as for reception. | */ | struct dccp_skb_cb { | + union { | + struct inet_skb_parm h4; | +#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE) | + struct inet6_skb_parm h6; | +#endif | + } header; | __u8 dccpd_type:4; | __u8 dccpd_ccval:4; | __u8 dccpd_reset_code, | diff --git a/net/dccp/proto.c b/net/dccp/proto.c | index e3f5d37..c91d3c1 100644 | --- a/net/dccp/proto.c | +++ b/net/dccp/proto.c | @@ -1057,6 +1057,9 @@ static int __init dccp_init(void) | int ehash_order, bhash_order, i; | int rc = -ENOBUFS; | | + BUILD_BUG_ON(sizeof(struct dccp_skb_cb) > | + FIELD_SIZEOF(struct sk_buff, cb)); | + | dccp_hashinfo.bind_bucket_cachep = | kmem_cache_create("dccp_bind_bucket", | sizeof(struct inet_bind_bucket), 0, -- The University of Aberdeen is a charity registered in Scotland, No SC013683. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [DCCP]: Fix skb->cb conflicts with IP 2008-04-04 13:25 ` Gerrit Renker @ 2008-04-04 13:40 ` Patrick McHardy 2008-04-04 13:47 ` Arnaldo Carvalho de Melo 1 sibling, 0 replies; 11+ messages in thread From: Patrick McHardy @ 2008-04-04 13:40 UTC (permalink / raw) To: Gerrit Renker, Patrick McHardy, acme, dccp, Linux Netdev List Gerrit Renker wrote: > Arnaldo, > > just a thought - I recall that there used to be a bug related to this, > which required to insert the following before sending an skb: > > memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)) > > This was about 1+1/2 .. 2 years ago and lead to crashes when the memset > was removed. Maybe with this solution the memsets are then no longer > necessary? The reference is > * output.c:dccp_transmit_skb() > * ipv4.c:dccp_v4_send_response() Yes, that shouldn't be needed anymore with this patch. ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [DCCP]: Fix skb->cb conflicts with IP 2008-04-04 13:25 ` Gerrit Renker 2008-04-04 13:40 ` Patrick McHardy @ 2008-04-04 13:47 ` Arnaldo Carvalho de Melo 1 sibling, 0 replies; 11+ messages in thread From: Arnaldo Carvalho de Melo @ 2008-04-04 13:47 UTC (permalink / raw) To: Gerrit Renker, Patrick McHardy, dccp, Linux Netdev List Em Fri, Apr 04, 2008 at 02:25:25PM +0100, Gerrit Renker escreveu: > Arnaldo, > > just a thought - I recall that there used to be a bug related to this, > which required to insert the following before sending an skb: > > memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)) > > This was about 1+1/2 .. 2 years ago and lead to crashes when the memset > was removed. Maybe with this solution the memsets are then no longer > necessary? The reference is > * output.c:dccp_transmit_skb() > * ipv4.c:dccp_v4_send_response() Well spotted, yes, those can now be safely removed, since we don't touch the initial inet6?_skb_parm area it will remain as zeros (alloc_skb did that for us) and we don't have to zero it anymore before passing it to IP. - Arnaldo > Gerrit > > | commit eced67957ee99f7b5fafdc73a58bcd037a1789b2 > | Author: Patrick McHardy <kaber@trash.net> > | Date: Fri Apr 4 14:10:23 2008 +0200 > | > | [DCCP]: Fix skb->cb conflicts with IP > | > | dev_queue_xmit() and the other IP output functions expect to get a skb > | with clear or properly initialized skb->cb. Unlike TCP and UDP, the > | dccp_skb_cb doesn't contain a struct inet_skb_parm at the beginning, > | so the DCCP-specific data is interpreted by the IP output functions. > | This can cause false negatives for the conditional POST_ROUTING hook > | invocation, making the packet bypass the hook. > | > | Add a inet_skb_parm/inet6_skb_parm union to the beginning of > | dccp_skb_cb to avoid clashes. Also add a BUILD_BUG_ON to make > | sure it fits in the cb. > | > | Signed-off-by: Patrick McHardy <kaber@trash.net> > | > | diff --git a/net/dccp/dccp.h b/net/dccp/dccp.h > | index fe7726b..f44d492 100644 > | --- a/net/dccp/dccp.h > | +++ b/net/dccp/dccp.h > | @@ -325,6 +325,12 @@ static inline int dccp_bad_service_code(const struct sock *sk, > | * This is used for transmission as well as for reception. > | */ > | struct dccp_skb_cb { > | + union { > | + struct inet_skb_parm h4; > | +#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE) > | + struct inet6_skb_parm h6; > | +#endif > | + } header; > | __u8 dccpd_type:4; > | __u8 dccpd_ccval:4; > | __u8 dccpd_reset_code, > | diff --git a/net/dccp/proto.c b/net/dccp/proto.c > | index e3f5d37..c91d3c1 100644 > | --- a/net/dccp/proto.c > | +++ b/net/dccp/proto.c > | @@ -1057,6 +1057,9 @@ static int __init dccp_init(void) > | int ehash_order, bhash_order, i; > | int rc = -ENOBUFS; > | > | + BUILD_BUG_ON(sizeof(struct dccp_skb_cb) > > | + FIELD_SIZEOF(struct sk_buff, cb)); > | + > | dccp_hashinfo.bind_bucket_cachep = > | kmem_cache_create("dccp_bind_bucket", > | sizeof(struct inet_bind_bucket), 0, > > > -- > > > The University of Aberdeen is a charity registered in Scotland, No SC013683. > ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [DCCP]: Fix skb->cb conflicts with IP 2008-04-04 12:13 [DCCP]: Fix skb->cb conflicts with IP Patrick McHardy 2008-04-04 13:25 ` Gerrit Renker @ 2008-04-04 13:26 ` Arnaldo Carvalho de Melo 2008-04-11 13:41 ` Patrick McHardy 1 sibling, 1 reply; 11+ messages in thread From: Arnaldo Carvalho de Melo @ 2008-04-04 13:26 UTC (permalink / raw) To: Patrick McHardy; +Cc: dccp, Linux Netdev List Em Fri, Apr 04, 2008 at 02:13:16PM +0200, Patrick McHardy escreveu: > commit eced67957ee99f7b5fafdc73a58bcd037a1789b2 > Author: Patrick McHardy <kaber@trash.net> > Date: Fri Apr 4 14:10:23 2008 +0200 > > [DCCP]: Fix skb->cb conflicts with IP > > dev_queue_xmit() and the other IP output functions expect to get a skb > with clear or properly initialized skb->cb. Unlike TCP and UDP, the > dccp_skb_cb doesn't contain a struct inet_skb_parm at the beginning, > so the DCCP-specific data is interpreted by the IP output functions. > This can cause false negatives for the conditional POST_ROUTING hook > invocation, making the packet bypass the hook. > > Add a inet_skb_parm/inet6_skb_parm union to the beginning of > dccp_skb_cb to avoid clashes. Also add a BUILD_BUG_ON to make > sure it fits in the cb. > > Signed-off-by: Patrick McHardy <kaber@trash.net> Thanks Patrick, Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com> ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [DCCP]: Fix skb->cb conflicts with IP 2008-04-04 13:26 ` Arnaldo Carvalho de Melo @ 2008-04-11 13:41 ` Patrick McHardy 2008-04-11 13:59 ` Gerrit Renker 0 siblings, 1 reply; 11+ messages in thread From: Patrick McHardy @ 2008-04-11 13:41 UTC (permalink / raw) To: David S. Miller; +Cc: Arnaldo Carvalho de Melo, dccp, Linux Netdev List [-- Attachment #1: Type: text/plain, Size: 1216 bytes --] Arnaldo Carvalho de Melo wrote: > Em Fri, Apr 04, 2008 at 02:13:16PM +0200, Patrick McHardy escreveu: > >> commit eced67957ee99f7b5fafdc73a58bcd037a1789b2 >> Author: Patrick McHardy <kaber@trash.net> >> Date: Fri Apr 4 14:10:23 2008 +0200 >> >> [DCCP]: Fix skb->cb conflicts with IP >> >> dev_queue_xmit() and the other IP output functions expect to get a skb >> with clear or properly initialized skb->cb. Unlike TCP and UDP, the >> dccp_skb_cb doesn't contain a struct inet_skb_parm at the beginning, >> so the DCCP-specific data is interpreted by the IP output functions. >> This can cause false negatives for the conditional POST_ROUTING hook >> invocation, making the packet bypass the hook. >> >> Add a inet_skb_parm/inet6_skb_parm union to the beginning of >> dccp_skb_cb to avoid clashes. Also add a BUILD_BUG_ON to make >> sure it fits in the cb. >> >> Signed-off-by: Patrick McHardy <kaber@trash.net> > > Thanks Patrick, > > Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com> Dave, I'm not sure whether you've missed this or expect it to go through Arnaldo, just want to make sure it doesn't get missed because of a misunderstanding :) [-- Attachment #2: x --] [-- Type: text/plain, Size: 1773 bytes --] commit eced67957ee99f7b5fafdc73a58bcd037a1789b2 Author: Patrick McHardy <kaber@trash.net> Date: Fri Apr 4 14:10:23 2008 +0200 [DCCP]: Fix skb->cb conflicts with IP dev_queue_xmit() and the other IP output functions expect to get a skb with clear or properly initialized skb->cb. Unlike TCP and UDP, the dccp_skb_cb doesn't contain a struct inet_skb_parm at the beginning, so the DCCP-specific data is interpreted by the IP output functions. This can cause false negatives for the conditional POST_ROUTING hook invocation, making the packet bypass the hook. Add a inet_skb_parm/inet6_skb_parm union to the beginning of dccp_skb_cb to avoid clashes. Also add a BUILD_BUG_ON to make sure it fits in the cb. Signed-off-by: Patrick McHardy <kaber@trash.net> diff --git a/net/dccp/dccp.h b/net/dccp/dccp.h index fe7726b..f44d492 100644 --- a/net/dccp/dccp.h +++ b/net/dccp/dccp.h @@ -325,6 +325,12 @@ static inline int dccp_bad_service_code(const struct sock *sk, * This is used for transmission as well as for reception. */ struct dccp_skb_cb { + union { + struct inet_skb_parm h4; +#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE) + struct inet6_skb_parm h6; +#endif + } header; __u8 dccpd_type:4; __u8 dccpd_ccval:4; __u8 dccpd_reset_code, diff --git a/net/dccp/proto.c b/net/dccp/proto.c index e3f5d37..c91d3c1 100644 --- a/net/dccp/proto.c +++ b/net/dccp/proto.c @@ -1057,6 +1057,9 @@ static int __init dccp_init(void) int ehash_order, bhash_order, i; int rc = -ENOBUFS; + BUILD_BUG_ON(sizeof(struct dccp_skb_cb) > + FIELD_SIZEOF(struct sk_buff, cb)); + dccp_hashinfo.bind_bucket_cachep = kmem_cache_create("dccp_bind_bucket", sizeof(struct inet_bind_bucket), 0, ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [DCCP]: Fix skb->cb conflicts with IP 2008-04-11 13:41 ` Patrick McHardy @ 2008-04-11 13:59 ` Gerrit Renker 2008-04-11 14:03 ` Patrick McHardy 0 siblings, 1 reply; 11+ messages in thread From: Gerrit Renker @ 2008-04-11 13:59 UTC (permalink / raw) To: Patrick McHardy Cc: David S. Miller, Arnaldo Carvalho de Melo, dccp, Linux Netdev List [-- Attachment #1: Type: text/plain, Size: 508 bytes --] >>> [DCCP]: Fix skb->cb conflicts with IP >>> dev_queue_xmit() and the other IP output functions expect to <snip> > > Dave, I'm not sure whether you've missed this or expect it > to go through Arnaldo, just want to make sure it doesn't > get missed because of a misunderstanding :) > > If it is not too much work, can you please also remove the two attached hunks which are now redundant thanks to the above patch. The University of Aberdeen is a charity registered in Scotland, No SC013683. [-- Attachment #2: remove_unused_memset.diff --] [-- Type: text/x-diff, Size: 714 bytes --] --- a/net/dccp/ipv4.c +++ b/net/dccp/ipv4.c @@ -489,7 +489,6 @@ static int dccp_v4_send_response(struct sock *sk, struct request_sock *req, dh->dccph_checksum = dccp_v4_csum_finish(skb, ireq->loc_addr, ireq->rmt_addr); - memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)); err = ip_build_and_send_pkt(skb, sk, ireq->loc_addr, ireq->rmt_addr, ireq->opt); --- a/net/dccp/output.c +++ b/net/dccp/output.c @@ -126,7 +126,6 @@ static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb) DCCP_INC_STATS(DCCP_MIB_OUTSEGS); - memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)); err = icsk->icsk_af_ops->queue_xmit(skb, 0); return net_xmit_eval(err); } ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [DCCP]: Fix skb->cb conflicts with IP 2008-04-11 13:59 ` Gerrit Renker @ 2008-04-11 14:03 ` Patrick McHardy 2008-04-11 14:05 ` Arnaldo Carvalho de Melo 0 siblings, 1 reply; 11+ messages in thread From: Patrick McHardy @ 2008-04-11 14:03 UTC (permalink / raw) To: Gerrit Renker, David S. Miller, Arnaldo Carvalho de Melo, dccp, Linux Netdev List [-- Attachment #1: Type: text/plain, Size: 499 bytes --] Gerrit Renker wrote: >>>> [DCCP]: Fix skb->cb conflicts with IP >>>> dev_queue_xmit() and the other IP output functions expect to > <snip> >> Dave, I'm not sure whether you've missed this or expect it >> to go through Arnaldo, just want to make sure it doesn't >> get missed because of a misunderstanding :) >> >> > If it is not too much work, can you please also remove the two attached > hunks which are now redundant thanks to the above patch. I've added it to the patch, thanks. [-- Attachment #2: x --] [-- Type: text/plain, Size: 2585 bytes --] commit eced67957ee99f7b5fafdc73a58bcd037a1789b2 Author: Patrick McHardy <kaber@trash.net> Date: Fri Apr 4 14:10:23 2008 +0200 [DCCP]: Fix skb->cb conflicts with IP dev_queue_xmit() and the other IP output functions expect to get a skb with clear or properly initialized skb->cb. Unlike TCP and UDP, the dccp_skb_cb doesn't contain a struct inet_skb_parm at the beginning, so the DCCP-specific data is interpreted by the IP output functions. This can cause false negatives for the conditional POST_ROUTING hook invocation, making the packet bypass the hook. Add a inet_skb_parm/inet6_skb_parm union to the beginning of dccp_skb_cb to avoid clashes. Also add a BUILD_BUG_ON to make sure it fits in the cb. [ Combined with patch from Gerrit Renker to remove two now unnecessary memsets of IPCB(skb)->opt ] Signed-off-by: Patrick McHardy <kaber@trash.net> diff --git a/net/dccp/dccp.h b/net/dccp/dccp.h index fe7726b..f44d492 100644 --- a/net/dccp/dccp.h +++ b/net/dccp/dccp.h @@ -325,6 +325,12 @@ static inline int dccp_bad_service_code(const struct sock *sk, * This is used for transmission as well as for reception. */ struct dccp_skb_cb { + union { + struct inet_skb_parm h4; +#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE) + struct inet6_skb_parm h6; +#endif + } header; __u8 dccpd_type:4; __u8 dccpd_ccval:4; __u8 dccpd_reset_code, diff --git a/net/dccp/proto.c b/net/dccp/proto.c index e3f5d37..c91d3c1 100644 --- a/net/dccp/proto.c +++ b/net/dccp/proto.c @@ -1057,6 +1057,9 @@ static int __init dccp_init(void) int ehash_order, bhash_order, i; int rc = -ENOBUFS; + BUILD_BUG_ON(sizeof(struct dccp_skb_cb) > + FIELD_SIZEOF(struct sk_buff, cb)); + dccp_hashinfo.bind_bucket_cachep = kmem_cache_create("dccp_bind_bucket", sizeof(struct inet_bind_bucket), 0, --- a/net/dccp/ipv4.c +++ b/net/dccp/ipv4.c @@ -489,7 +489,6 @@ static int dccp_v4_send_response(struct sock *sk, struct request_sock *req, dh->dccph_checksum = dccp_v4_csum_finish(skb, ireq->loc_addr, ireq->rmt_addr); - memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)); err = ip_build_and_send_pkt(skb, sk, ireq->loc_addr, ireq->rmt_addr, ireq->opt); --- a/net/dccp/output.c +++ b/net/dccp/output.c @@ -126,7 +126,6 @@ static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb) DCCP_INC_STATS(DCCP_MIB_OUTSEGS); - memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)); err = icsk->icsk_af_ops->queue_xmit(skb, 0); return net_xmit_eval(err); } ^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [DCCP]: Fix skb->cb conflicts with IP 2008-04-11 14:03 ` Patrick McHardy @ 2008-04-11 14:05 ` Arnaldo Carvalho de Melo 2008-04-11 18:24 ` David Miller 2008-04-13 1:35 ` David Miller 0 siblings, 2 replies; 11+ messages in thread From: Arnaldo Carvalho de Melo @ 2008-04-11 14:05 UTC (permalink / raw) To: Patrick McHardy Cc: Gerrit Renker, David S. Miller, Arnaldo Carvalho de Melo, dccp, Linux Netdev List Em Fri, Apr 11, 2008 at 04:03:10PM +0200, Patrick McHardy escreveu: > Gerrit Renker wrote: >>>>> [DCCP]: Fix skb->cb conflicts with IP >>>>> dev_queue_xmit() and the other IP output functions expect to >> <snip> >>> Dave, I'm not sure whether you've missed this or expect it >>> to go through Arnaldo, just want to make sure it doesn't >>> get missed because of a misunderstanding :) >>> >>> >> If it is not too much work, can you please also remove the two attached >> hunks which are now redundant thanks to the above patch. > > > I've added it to the patch, thanks. Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com> > commit eced67957ee99f7b5fafdc73a58bcd037a1789b2 > Author: Patrick McHardy <kaber@trash.net> > Date: Fri Apr 4 14:10:23 2008 +0200 > > [DCCP]: Fix skb->cb conflicts with IP > > dev_queue_xmit() and the other IP output functions expect to get a skb > with clear or properly initialized skb->cb. Unlike TCP and UDP, the > dccp_skb_cb doesn't contain a struct inet_skb_parm at the beginning, > so the DCCP-specific data is interpreted by the IP output functions. > This can cause false negatives for the conditional POST_ROUTING hook > invocation, making the packet bypass the hook. > > Add a inet_skb_parm/inet6_skb_parm union to the beginning of > dccp_skb_cb to avoid clashes. Also add a BUILD_BUG_ON to make > sure it fits in the cb. > > [ Combined with patch from Gerrit Renker to remove two now unnecessary > memsets of IPCB(skb)->opt ] > > Signed-off-by: Patrick McHardy <kaber@trash.net> > > diff --git a/net/dccp/dccp.h b/net/dccp/dccp.h > index fe7726b..f44d492 100644 > --- a/net/dccp/dccp.h > +++ b/net/dccp/dccp.h > @@ -325,6 +325,12 @@ static inline int dccp_bad_service_code(const struct sock *sk, > * This is used for transmission as well as for reception. > */ > struct dccp_skb_cb { > + union { > + struct inet_skb_parm h4; > +#if defined(CONFIG_IPV6) || defined (CONFIG_IPV6_MODULE) > + struct inet6_skb_parm h6; > +#endif > + } header; > __u8 dccpd_type:4; > __u8 dccpd_ccval:4; > __u8 dccpd_reset_code, > diff --git a/net/dccp/proto.c b/net/dccp/proto.c > index e3f5d37..c91d3c1 100644 > --- a/net/dccp/proto.c > +++ b/net/dccp/proto.c > @@ -1057,6 +1057,9 @@ static int __init dccp_init(void) > int ehash_order, bhash_order, i; > int rc = -ENOBUFS; > > + BUILD_BUG_ON(sizeof(struct dccp_skb_cb) > > + FIELD_SIZEOF(struct sk_buff, cb)); > + > dccp_hashinfo.bind_bucket_cachep = > kmem_cache_create("dccp_bind_bucket", > sizeof(struct inet_bind_bucket), 0, > --- a/net/dccp/ipv4.c > +++ b/net/dccp/ipv4.c > @@ -489,7 +489,6 @@ static int dccp_v4_send_response(struct sock *sk, struct request_sock *req, > > dh->dccph_checksum = dccp_v4_csum_finish(skb, ireq->loc_addr, > ireq->rmt_addr); > - memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)); > err = ip_build_and_send_pkt(skb, sk, ireq->loc_addr, > ireq->rmt_addr, > ireq->opt); > --- a/net/dccp/output.c > +++ b/net/dccp/output.c > @@ -126,7 +126,6 @@ static int dccp_transmit_skb(struct sock *sk, struct sk_buff *skb) > > DCCP_INC_STATS(DCCP_MIB_OUTSEGS); > > - memset(&(IPCB(skb)->opt), 0, sizeof(IPCB(skb)->opt)); > err = icsk->icsk_af_ops->queue_xmit(skb, 0); > return net_xmit_eval(err); > } ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [DCCP]: Fix skb->cb conflicts with IP 2008-04-11 14:05 ` Arnaldo Carvalho de Melo @ 2008-04-11 18:24 ` David Miller 2008-04-13 1:35 ` David Miller 1 sibling, 0 replies; 11+ messages in thread From: David Miller @ 2008-04-11 18:24 UTC (permalink / raw) To: acme; +Cc: kaber, gerrit, dccp, netdev From: Arnaldo Carvalho de Melo <acme@redhat.com> Date: Fri, 11 Apr 2008 11:05:01 -0300 > Em Fri, Apr 11, 2008 at 04:03:10PM +0200, Patrick McHardy escreveu: > > Gerrit Renker wrote: > >>>>> [DCCP]: Fix skb->cb conflicts with IP > >>>>> dev_queue_xmit() and the other IP output functions expect to > >> <snip> > >>> Dave, I'm not sure whether you've missed this or expect it > >>> to go through Arnaldo, just want to make sure it doesn't > >>> get missed because of a misunderstanding :) > >>> > >>> > >> If it is not too much work, can you please also remove the two attached > >> hunks which are now redundant thanks to the above patch. > > > > > > I've added it to the patch, thanks. > > Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com> And for the record, I have it and will merge :-) ^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [DCCP]: Fix skb->cb conflicts with IP 2008-04-11 14:05 ` Arnaldo Carvalho de Melo 2008-04-11 18:24 ` David Miller @ 2008-04-13 1:35 ` David Miller 1 sibling, 0 replies; 11+ messages in thread From: David Miller @ 2008-04-13 1:35 UTC (permalink / raw) To: acme; +Cc: kaber, gerrit, dccp, netdev From: Arnaldo Carvalho de Melo <acme@redhat.com> Date: Fri, 11 Apr 2008 11:05:01 -0300 > Em Fri, Apr 11, 2008 at 04:03:10PM +0200, Patrick McHardy escreveu: > > Gerrit Renker wrote: > >>>>> [DCCP]: Fix skb->cb conflicts with IP > >>>>> dev_queue_xmit() and the other IP output functions expect to > >> <snip> > >>> Dave, I'm not sure whether you've missed this or expect it > >>> to go through Arnaldo, just want to make sure it doesn't > >>> get missed because of a misunderstanding :) > >>> > >>> > >> If it is not too much work, can you please also remove the two attached > >> hunks which are now redundant thanks to the above patch. > > > > > > I've added it to the patch, thanks. > > Acked-by: Arnaldo Carvalho de Melo <acme@redhat.com> Applied to net-2.6, thanks everyone. ^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2008-04-13 1:35 UTC | newest] Thread overview: 11+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2008-04-04 12:13 [DCCP]: Fix skb->cb conflicts with IP Patrick McHardy 2008-04-04 13:25 ` Gerrit Renker 2008-04-04 13:40 ` Patrick McHardy 2008-04-04 13:47 ` Arnaldo Carvalho de Melo 2008-04-04 13:26 ` Arnaldo Carvalho de Melo 2008-04-11 13:41 ` Patrick McHardy 2008-04-11 13:59 ` Gerrit Renker 2008-04-11 14:03 ` Patrick McHardy 2008-04-11 14:05 ` Arnaldo Carvalho de Melo 2008-04-11 18:24 ` David Miller 2008-04-13 1:35 ` David Miller
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).