DCCP: Initialize ireq6->pktopts before used it

ireq6->pktopts is not initialized after dccp_reqsk_init(), and it will be free in dccp_v6_reqsk_destructor(), so if dccp_parse_options() is fail, this may cause kernel panic since ireq6->pktopts is not initialized.

This patch fix this problem by initialize ireq6->pktopts before used it.

static void dccp_v6_reqsk_destructor(struct request_sock *req)

{
       dccp_feat_list_purge(&dccp_rsk(req)->dreq_featneg);
       if (inet6_rsk(req)->pktopts != NULL)
               kfree_skb(inet6_rsk(req)->pktopts);
}

Pid: 0, comm: swapper Not tainted (2.6.26-rc2 #1)
EIP: 0060:[<c05acdaf>] EFLAGS: 00010206 CPU: 0
EIP is at kfree_skb+0x9/0x30
EAX: 00002fde EBX: c7306e80 ECX: c7801080 EDX: 00002fde
ESI: c7983680 EDI: c72d9800 EBP: c075adfc ESP: c075adfc
DS: 007b ES: 007b FS: 00d8 GS: 0000 SS: 0068
Process swapper (pid: 0, tiÀ75a000 taskÀ6df3a0 task.tiÀ714000)

Stack: c075ae08 c8a259d8 c7a0f848 c075ae38 c8a260fc c7983680 c72d9800 
c72d9b90
     64000000 c79836a0 c7306e80 8cf2437f c7a0f848 c7983680 c72d9800 
c075ae78
     c89e6c78 c7983680 c72d9800 0a804500 c79836a0 0c011908 f24206cc 
c46c3660
Call Trace:
[<c8a259d8>] ? dccp_v6_reqsk_destructor+0x1f/0x22 [dccp_ipv6]
[<c8a260fc>] ? dccp_v6_conn_request+0x243/0x27d [dccp_ipv6]
[<c89e6c78>] ? dccp_rcv_state_process+0x3d/0x4b5 [dccp]
[<c8a25976>] ? dccp_v6_do_rcv+0x132/0x175 [dccp_ipv6]
[<c05bb355>] ? sk_filter+0x66/0x6d
[<c05ab5c2>] ? sk_receive_skb+0x32/0x7c
[<c8a267b3>] ? dccp_v6_rcv+0x2a5/0x32a [dccp_ipv6]
[<c8ee2ee0>] ? ip6_input_finish+0x158/0x280 [ipv6]
[<c8ee304a>] ? ip6_input+0x42/0x47 [ipv6]
[<c8ee3357>] ? ipv6_rcv+0x27c/0x2c9 [ipv6]
[<c05b1336>] ? netif_receive_skb+0x2e0/0x349
[<c88f2a12>] ? pcnet32_poll+0x333/0x66e [pcnet32]
[<c0438afa>] ? clocksource_watchdog+0x21e/0x22d
[<c040428b>] ? common_interrupt+0x23/0x28
[<c05b308c>] ? net_rx_action+0x8f/0x147
[<c0427c5b>] ? __do_softirq+0x64/0xcd
[<c0405898>] ? do_softirq+0x55/0x88
[<c0427bf5>] ? irq_exit+0x38/0x3a
[<c0412b42>] ? smp_apic_timer_interrupt+0x71/0x7f
[<c04025eb>] ? default_idle+0x0/0x42
[<c0404348>] ? apic_timer_interrupt+0x28/0x30
[<c04025eb>] ? default_idle+0x0/0x42
[<c0402618>] ? default_idle+0x2d/0x42
[<c0402566>] ? cpu_idle+0x8b/0x9f
[<c060c89a>] ? rest_init+0x4e/0x50

===========>

Signed-off-by Wei Yongjun <[email protected]>

net/dccp/ipv6.c 2008-06-05 05:58:00.000000000 -0400 renamed from net/dccp/ipv6.c 2008-05-29 22:27:55.000000000 -0400 +3 -2

@@ -413,6 +413,9 @@ static int dccp_v6_conn_request(struct s
    if (dccp_reqsk_init(req, dccp_sk(sk), skb))
        goto drop_and_free;
 
+    ireq6 = inet6_rsk(req);
+    ireq6->pktopts    = NULL;
+
    dreq = dccp_rsk(req);
    if (dccp_parse_options(sk, dreq, skb))
        goto drop_and_free;

@@ -420,10 +423,8 @@ static int dccp_v6_conn_request(struct s
    if (security_inet_conn_request(sk, skb, req))
        goto drop_and_free;
 
-    ireq6 = inet6_rsk(req);
    ipv6_addr_copy(&ireq6->rmt_addr, &ipv6_hdr(skb)->saddr);
    ipv6_addr_copy(&ireq6->loc_addr, &ipv6_hdr(skb)->daddr);
-    ireq6->pktopts    = NULL;
 
    if (ipv6_opt_accepted(sk, skb) ||
        np->rxopt.bits.rxinfo || np->rxopt.bits.rxoinfo ||

Signature · 4 lines

-- 
To unsubscribe from this list: send the line "unsubscribe dccp" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

I think I should add dccp_v6_reqsk_init() do to the init work since dccp_reqsk_init may be fail too.

Quoted 4 lines

I think I should add dccp_v6_reqsk_init() do to the init work since dccp_reqsk_init may be fail too.

I think this is not necessary, it can be done like this

	req = inet6_reqsk_alloc(&dccp6_request_sock_ops);
	if (req = NULL)
		goto drop;

	ireq6 = inet6_rsk(req);
	ireq6->pktopts = NULL;

	if (dccp_reqsk_init(req, dccp_sk(sk), skb))
		goto drop_and_free;

This is since dccp_reqsk_init() only initialises the inet_sk and dccp_sk parts, and does not do IPv6-specific initialisation.

Irrespective of the oops, this is an error and will be fixed in the test tree today.

With regard to the oops, the log pointed to the dccp_v6_reqsk_destructor and so it would make sense, since the pktopts was not initialised to NULL and since kfree_skb() calls skb->destructor().

As before, thanks a lot for testing this code and for reporting this.

Quoted 4 lines

I think I should add dccp_v6_reqsk_init() do to the init work since dccp_reqsk_init may be fail too.

I think this is not necessary, it can be done like this

	req = inet6_reqsk_alloc(&dccp6_request_sock_ops);
	if (req = NULL)
		goto drop;

	ireq6 = inet6_rsk(req);
	ireq6->pktopts = NULL;

	if (dccp_reqsk_init(req, dccp_sk(sk), skb))
		goto drop_and_free;

This is since dccp_reqsk_init() only initialises the inet_sk and dccp_sk parts, and does not do IPv6-specific initialisation.

Irrespective of the oops, this is an error and will be fixed in the test tree today.

With regard to the oops, the log pointed to the dccp_v6_reqsk_destructor and so it would make sense, since the pktopts was not initialised to NULL and since kfree_skb() calls skb->destructor().

As before, thanks a lot for testing this code and for reporting this.

I have dug out the patch to which this applies. Since it is a bug-fix, I would much rather like to merge your patch with the existing one, as done below.

Wei, can you please have a look at it and add your Signed-Off if you are ok with it?

Incidentally, your patch also fixes a second problem: the same problem would occur if security_inet_conn_request(sk, skb, req) failed; but it was apparently not triggered so far; this is also avoided below.

If there is no disagreement, I will upload this to the test tree within the next hour.

-----------------------> Patch v2 (in test tree) <----------------------
dccp: Socket support for feature negotiation and its initialisation

This provides feature-negotiation initialisation for both DCCP sockets and DCCP request_sockets, to support feature negotiation during connection setup.

It also resolves a FIXME regarding the congestion control initialisation.

Signed-off-by Gerrit Renker <[email protected]>

Acked-by Ian McDonald <[email protected]>

---
 include/linux/dccp.h |    4 ++++
 net/dccp/dccp.h      |    3 ++-
 net/dccp/feat.c      |   19 +++++++++++++++++++
 net/dccp/feat.h      |    1 +
 net/dccp/input.c     |    2 --
 net/dccp/ipv4.c      |    3 ++-
 net/dccp/ipv6.c      |    8 +++++---
 net/dccp/minisocks.c |    7 ++++++-
 net/dccp/proto.c     |    1 +
 9 files changed, 40 insertions(+), 8 deletions(-)

net/dccp/ipv6.c +5 -3

@@ -409,7 +409,11 @@ static int dccp_v6_conn_request(struct s
 	if (req = NULL)
 		goto drop;
 
-	dccp_reqsk_init(req, skb);
+	ireq6 = inet6_rsk(req);
+	ireq6->pktopts = NULL;
+
+	if (dccp_reqsk_init(req, dccp_sk(sk), skb))
+		goto drop_and_free;
 
 	dreq = dccp_rsk(req);
 	if (dccp_parse_options(sk, dreq, skb))

@@ -418,10 +422,8 @@ static int dccp_v6_conn_request(struct s
 	if (security_inet_conn_request(sk, skb, req))
 		goto drop_and_free;
 
-	ireq6 = inet6_rsk(req);
 	ipv6_addr_copy(&ireq6->rmt_addr, &ipv6_hdr(skb)->saddr);
 	ipv6_addr_copy(&ireq6->loc_addr, &ipv6_hdr(skb)->daddr);
-	ireq6->pktopts	= NULL;
 
 	if (ipv6_opt_accepted(sk, skb) ||
 	    np->rxopt.bits.rxinfo || np->rxopt.bits.rxoinfo ||

net/dccp/input.c +0 -2

@@ -590,8 +590,6 @@ int dccp_rcv_state_process(struct sock *
 			if (inet_csk(sk)->icsk_af_ops->conn_request(sk,
 								    skb) < 0)
 				return 1;
-
-			/* FIXME: do congestion control initialization */
 			goto discard;
 		}
 		if (dh->dccph_type = DCCP_PKT_RESET)

include/linux/dccp.h +4 -0

@@ -416,6 +416,7 @@ extern void dccp_minisock_init(struct dc
  * @dreq_iss: initial sequence number sent on the Response (RFC 4340, 7.1)
  * @dreq_isr: initial sequence number received on the Request
  * @dreq_service: service code present on the Request (there is just one)
+ * @dreq_featneg: feature negotiation options for this connection
  * The following two fields are analogous to the ones in dccp_sock:
  * @dreq_timestamp_echo: last received timestamp to echo (13.1)
  * @dreq_timestamp_echo: the time of receiving the last @dreq_timestamp_echo

@@ -425,6 +426,7 @@ struct dccp_request_sock {
 	__u64			 dreq_iss;
 	__u64			 dreq_isr;
 	__be32			 dreq_service;
+	struct list_head	 dreq_featneg;
 	__u32			 dreq_timestamp_echo;
 	__u32			 dreq_timestamp_time;
 };

@@ -502,6 +504,7 @@ struct dccp_ackvec;
  * @dccps_mss_cache - current value of MSS (path MTU minus header sizes)
  * @dccps_rate_last - timestamp for rate-limiting DCCP-Sync (RFC 4340, 7.5.4)
  * @dccps_minisock - associated minisock (accessed via dccp_msk)
+ * @dccps_featneg - tracks feature-negotiation state (mostly during handshake)
  * @dccps_hc_rx_ackvec - rx half connection ack vector
  * @dccps_hc_rx_ccid - CCID used for the receiver (or receiving half-connection)
  * @dccps_hc_tx_ccid - CCID used for the sender (or sending half-connection)

@@ -539,6 +542,7 @@ struct dccp_sock {
 	unsigned long			dccps_ndp_count;
 	unsigned long			dccps_rate_last;
 	struct dccp_minisock		dccps_minisock;
+	struct list_head		dccps_featneg;
 	struct dccp_ackvec		*dccps_hc_rx_ackvec;
 	struct ccid			*dccps_hc_rx_ccid;
 	struct ccid			*dccps_hc_tx_ccid;

net/dccp/proto.c +1 -0

@@ -193,6 +193,7 @@ int dccp_init_sock(struct sock *sk, cons
 
 	dccp_init_xmit_timers(sk);
 
+	INIT_LIST_HEAD(&dp->dccps_featneg);
 	/*
 	 * FIXME: We're hardcoding the CCID, and doing this at this point makes
 	 * the listening (master) sock get CCID control blocks, which is not

net/dccp/feat.h +1 -0

@@ -95,6 +95,7 @@ extern int  dccp_feat_confirm_recv(struc
 				   u8 *val, u8 len);
 extern void dccp_feat_clean(struct dccp_minisock *dmsk);
 extern int  dccp_feat_clone(struct sock *oldsk, struct sock *newsk);
+extern int  dccp_feat_clone_list(struct list_head const *, struct list_head *);
 extern int  dccp_feat_init(struct dccp_minisock *dmsk);
 
 #endif /* _DCCP_FEAT_H */

net/dccp/feat.c +19 -0

@@ -272,6 +272,25 @@ void dccp_feat_list_purge(struct list_he
 }
 EXPORT_SYMBOL_GPL(dccp_feat_list_purge);
 
+/* generate @to as full clone of @from - @to must not contain any nodes */
+int dccp_feat_clone_list(struct list_head const *from, struct list_head *to)
+{
+	struct dccp_feat_entry *entry, *new;
+
+	INIT_LIST_HEAD(to);
+	list_for_each_entry(entry, from, node) {
+		new = dccp_feat_clone_entry(entry);
+		if (new = NULL)
+			goto cloning_failed;
+		list_add_tail(&new->node, to);
+	}
+	return 0;
+
+cloning_failed:
+	dccp_feat_list_purge(to);
+	return -ENOMEM;
+}
+
 int dccp_feat_change(struct dccp_minisock *dmsk, u8 type, u8 feature,
 		     u8 *val, u8 len, gfp_t gfp)
 {

net/dccp/dccp.h +2 -1

@@ -236,7 +236,8 @@ extern const char *dccp_state_name(const
 extern void dccp_set_state(struct sock *sk, const int state);
 extern void dccp_done(struct sock *sk);
 
-extern void dccp_reqsk_init(struct request_sock *req, struct sk_buff *skb);
+extern int  dccp_reqsk_init(struct request_sock *rq, struct dccp_sock const *dp,
+			    struct sk_buff const *skb);
 
 extern int dccp_v4_conn_request(struct sock *sk, struct sk_buff *skb);
 

net/dccp/minisocks.c +3 -1

@@ -125,6 +125,7 @@ struct sock *dccp_create_openreq_child(s
 		newdp->dccps_timestamp_time = dreq->dreq_timestamp_time;
 		newicsk->icsk_rto	    = DCCP_TIMEOUT_INIT;
 
+		INIT_LIST_HEAD(&newdp->dccps_featneg);
 		if (dccp_feat_clone(sk, newsk))
 			goto out_free;
 

@@ -303,7 +304,8 @@ void dccp_reqsk_send_ack(struct sk_buff 
 
 EXPORT_SYMBOL_GPL(dccp_reqsk_send_ack);
 
-void dccp_reqsk_init(struct request_sock *req, struct sk_buff *skb)
+int dccp_reqsk_init(struct request_sock *req,
+		    struct dccp_sock const *dp, struct sk_buff const *skb)
 {
 	struct dccp_request_sock *dreq = dccp_rsk(req);
 

net/dccp/minisocks.c +3 -0

+
+	/* inherit feature negotiation options from listening socket */
+	return dccp_feat_clone_list(&dp->dccps_featneg, &dreq->dreq_featneg);
 }
 

EXPORT_SYMBOL_GPL(dccp_reqsk_init);

net/dccp/ipv4.c +2 -1

@@ -593,7 +593,8 @@ int dccp_v4_conn_request(struct sock *sk
 	if (req = NULL)
 		goto drop;
 
-	dccp_reqsk_init(req, skb);
+	if (dccp_reqsk_init(req, dccp_sk(sk), skb))
+		goto drop_and_free;
 
 	dreq = dccp_rsk(req);
 	if (dccp_parse_options(sk, dreq, skb))

IPv4 may have the same problem althought it is hard to test it.

[PATCH] DCCP: Initialize inet_rsk(req)->opt before used it dccp_reqsk_init() does not initialises the inet_rsk(req)->opt which will be used in dccp_v4_reqsk_destructor().

This patch fixed this problem.