netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [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 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: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 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).