Netdev List
 help / color / mirror / Atom feed
* [PATCH 1/4] netfilter: nfq_ct_hook needs __rcu and __read_mostly
From: pablo @ 2012-06-23 23:53 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1340495609-26250-1-git-send-email-pablo@netfilter.org>

From: Pablo Neira Ayuso <pablo@netfilter.org>

This removes some sparse warnings.

Reported-by: Fengguang Wu <wfg@linux.intel.com>
Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 include/linux/netfilter.h |    2 +-
 net/netfilter/core.c      |    2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/include/linux/netfilter.h b/include/linux/netfilter.h
index dca19e6..38b96a5 100644
--- a/include/linux/netfilter.h
+++ b/include/linux/netfilter.h
@@ -404,7 +404,7 @@ struct nfq_ct_hook {
 	void (*seq_adjust)(struct sk_buff *skb, struct nf_conn *ct,
 			   u32 ctinfo, int off);
 };
-extern struct nfq_ct_hook *nfq_ct_hook;
+extern struct nfq_ct_hook __rcu *nfq_ct_hook;
 #else
 static inline void nf_ct_attach(struct sk_buff *new, struct sk_buff *skb) {}
 #endif
diff --git a/net/netfilter/core.c b/net/netfilter/core.c
index 7eef845..4cd10ed 100644
--- a/net/netfilter/core.c
+++ b/net/netfilter/core.c
@@ -265,7 +265,7 @@ void nf_conntrack_destroy(struct nf_conntrack *nfct)
 }
 EXPORT_SYMBOL(nf_conntrack_destroy);
 
-struct nfq_ct_hook *nfq_ct_hook;
+struct nfq_ct_hook __rcu *nfq_ct_hook __read_mostly;
 EXPORT_SYMBOL_GPL(nfq_ct_hook);
 
 #endif /* CONFIG_NF_CONNTRACK */
-- 
1.7.10

^ permalink raw reply related

* [PATCH 3/4] netfilter: nfnetlink_queue: fix sparse warning due to missing include
From: pablo @ 2012-06-23 23:53 UTC (permalink / raw)
  To: netfilter-devel; +Cc: davem, netdev
In-Reply-To: <1340495609-26250-1-git-send-email-pablo@netfilter.org>

From: Pablo Neira Ayuso <pablo@netfilter.org>

This patch fixes a sparse warning due to missing include header file.

Signed-off-by: Pablo Neira Ayuso <pablo@netfilter.org>
---
 net/netfilter/nfnetlink_queue_ct.c |    1 +
 1 file changed, 1 insertion(+)

diff --git a/net/netfilter/nfnetlink_queue_ct.c b/net/netfilter/nfnetlink_queue_ct.c
index 01247b7..ab61d66 100644
--- a/net/netfilter/nfnetlink_queue_ct.c
+++ b/net/netfilter/nfnetlink_queue_ct.c
@@ -12,6 +12,7 @@
 #include <linux/netfilter/nfnetlink.h>
 #include <linux/netfilter/nfnetlink_queue.h>
 #include <net/netfilter/nf_conntrack.h>
+#include <net/netfilter/nfnetlink_queue.h>
 
 struct nf_conn *nfqnl_ct_get(struct sk_buff *entskb, size_t *size,
 			     enum ip_conntrack_info *ctinfo)
-- 
1.7.10

^ permalink raw reply related

* Re: [PATCH 0/4] netfilter updates for net-next (upcoming 3.6), batch 4
From: David Miller @ 2012-06-24  0:14 UTC (permalink / raw)
  To: pablo; +Cc: netfilter-devel, netdev
In-Reply-To: <1340495609-26250-1-git-send-email-pablo@netfilter.org>

From: pablo@netfilter.org
Date: Sun, 24 Jun 2012 01:53:25 +0200

> From: Pablo Neira Ayuso <pablo@netfilter.org>
> 
> Hi David,
> 
> The following four patches provide Netfilter fixes for the cthelper
> infrastructure that was recently merged mainstream, they are:
> 
> * two fixes for compilation breakage with two different configurations:
> 
>   - CONFIG_NF_NAT=m and CONFIG_NF_CT_NETLINK=y
>   - NF_CONNTRACK_EVENTS=n and CONFIG_NETFILTER_NETLINK_QUEUE_CT=y
> 
> * two fixes for sparse warnings.
> 
> You can pull this changes from:
> 
> git://1984.lsi.us.es/nf-next master

Pulled, thanks Pablo.

^ permalink raw reply

* [PATCH net-next] tcp: Fix bug in tcp socket early demux
From: Vijay Subramanian @ 2012-06-24  3:38 UTC (permalink / raw)
  To: netdev; +Cc: davem, eric.dumazet, alexander.h.duyck, Vijay Subramanian

The dest port for the call to __inet_lookup_established() in TCP early demux
code is passed with the wrong endian-ness. This causes the lookup to fail
leading to early demux not being used.

Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
---
 net/ipv4/tcp_ipv4.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index b52934f..1781dc6 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1701,7 +1701,7 @@ int tcp_v4_early_demux(struct sk_buff *skb)
 	dev = skb->dev;
 	sk = __inet_lookup_established(net, &tcp_hashinfo,
 				       iph->saddr, th->source,
-				       iph->daddr, th->dest,
+				       iph->daddr, ntohs(th->dest),
 				       dev->ifindex);
 	if (sk) {
 		skb->sk = sk;
-- 
1.7.0.4

^ permalink raw reply related

* [PATCH 1/5] tcp: heed result of security_inet_conn_request() in tcp_v6_conn_request()
From: Neal Cardwell @ 2012-06-24  5:22 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Eric Dumazet, Tom Herbert, Neal Cardwell

If security_inet_conn_request() returns non-zero then TCP/IPv6 should
drop the request, just as in TCP/IPv4 and DCCP in both IPv4 and IPv6.

Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
 net/ipv6/tcp_ipv6.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 3a9aec2..9df64a5 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -1212,7 +1212,8 @@ have_isn:
 	tcp_rsk(req)->snt_isn = isn;
 	tcp_rsk(req)->snt_synack = tcp_time_stamp;
 
-	security_inet_conn_request(sk, skb, req);
+	if (security_inet_conn_request(sk, skb, req))
+		goto drop_and_release;
 
 	if (tcp_v6_send_synack(sk, req,
 			       (struct request_values *)&tmp_ext,
-- 
1.7.7.3

^ permalink raw reply related

* [PATCH 2/5] tcp: fix inet6_csk_route_req() for link-local addresses
From: Neal Cardwell @ 2012-06-24  5:22 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Eric Dumazet, Tom Herbert, Neal Cardwell
In-Reply-To: <1340515324-2152-1-git-send-email-ncardwell@google.com>

Fix inet6_csk_route_req() to use as the flowi6_oif the treq->iif,
which is correctly fixed up in tcp_v6_conn_request() to handle the
case of link-local addresses. This brings it in line with the
tcp_v6_send_synack() code, which is already correctly using the
treq->iif in this way.

Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
 net/ipv6/inet6_connection_sock.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
index e6cee52..e23d354 100644
--- a/net/ipv6/inet6_connection_sock.c
+++ b/net/ipv6/inet6_connection_sock.c
@@ -68,7 +68,7 @@ struct dst_entry *inet6_csk_route_req(struct sock *sk,
 	fl6.daddr = treq->rmt_addr;
 	final_p = fl6_update_dst(&fl6, np->opt, &final);
 	fl6.saddr = treq->loc_addr;
-	fl6.flowi6_oif = sk->sk_bound_dev_if;
+	fl6.flowi6_oif = treq->iif;
 	fl6.flowi6_mark = sk->sk_mark;
 	fl6.fl6_dport = inet_rsk(req)->rmt_port;
 	fl6.fl6_sport = inet_rsk(req)->loc_port;
-- 
1.7.7.3

^ permalink raw reply related

* [PATCH 3/5] tcp: pass fl6 to inet6_csk_route_req()
From: Neal Cardwell @ 2012-06-24  5:22 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Eric Dumazet, Tom Herbert, Neal Cardwell
In-Reply-To: <1340515324-2152-1-git-send-email-ncardwell@google.com>

This commit changes inet_csk_route_req() so that it uses a pointer to
a struct flowi6, rather than allocating its own on the stack. This
brings its behavior in line with its IPv4 cousin,
inet_csk_route_req(), and allows a follow-on patch to fix a dst leak.

Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
 include/net/inet6_connection_sock.h |    1 +
 net/ipv6/inet6_connection_sock.c    |   26 +++++++++++++-------------
 net/ipv6/tcp_ipv6.c                 |    9 ++++++---
 3 files changed, 20 insertions(+), 16 deletions(-)

diff --git a/include/net/inet6_connection_sock.h b/include/net/inet6_connection_sock.h
index 1866a67..df2a857 100644
--- a/include/net/inet6_connection_sock.h
+++ b/include/net/inet6_connection_sock.h
@@ -26,6 +26,7 @@ extern int inet6_csk_bind_conflict(const struct sock *sk,
 				   const struct inet_bind_bucket *tb, bool relax);
 
 extern struct dst_entry* inet6_csk_route_req(struct sock *sk,
+					     struct flowi6 *fl6,
 					     const struct request_sock *req);
 
 extern struct request_sock *inet6_csk_search_req(const struct sock *sk,
diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
index e23d354..bceb144 100644
--- a/net/ipv6/inet6_connection_sock.c
+++ b/net/ipv6/inet6_connection_sock.c
@@ -55,26 +55,26 @@ int inet6_csk_bind_conflict(const struct sock *sk,
 EXPORT_SYMBOL_GPL(inet6_csk_bind_conflict);
 
 struct dst_entry *inet6_csk_route_req(struct sock *sk,
+				      struct flowi6 *fl6,
 				      const struct request_sock *req)
 {
 	struct inet6_request_sock *treq = inet6_rsk(req);
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct in6_addr *final_p, final;
 	struct dst_entry *dst;
-	struct flowi6 fl6;
-
-	memset(&fl6, 0, sizeof(fl6));
-	fl6.flowi6_proto = IPPROTO_TCP;
-	fl6.daddr = treq->rmt_addr;
-	final_p = fl6_update_dst(&fl6, np->opt, &final);
-	fl6.saddr = treq->loc_addr;
-	fl6.flowi6_oif = treq->iif;
-	fl6.flowi6_mark = sk->sk_mark;
-	fl6.fl6_dport = inet_rsk(req)->rmt_port;
-	fl6.fl6_sport = inet_rsk(req)->loc_port;
-	security_req_classify_flow(req, flowi6_to_flowi(&fl6));
 
-	dst = ip6_dst_lookup_flow(sk, &fl6, final_p, false);
+	memset(fl6, 0, sizeof(*fl6));
+	fl6->flowi6_proto = IPPROTO_TCP;
+	fl6->daddr = treq->rmt_addr;
+	final_p = fl6_update_dst(fl6, np->opt, &final);
+	fl6->saddr = treq->loc_addr;
+	fl6->flowi6_oif = treq->iif;
+	fl6->flowi6_mark = sk->sk_mark;
+	fl6->fl6_dport = inet_rsk(req)->rmt_port;
+	fl6->fl6_sport = inet_rsk(req)->loc_port;
+	security_req_classify_flow(req, flowi6_to_flowi(fl6));
+
+	dst = ip6_dst_lookup_flow(sk, fl6, final_p, false);
 	if (IS_ERR(dst))
 		return NULL;
 
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 9df64a5..cfeefbf 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -475,7 +475,8 @@ out:
 }
 
 
-static int tcp_v6_send_synack(struct sock *sk, struct request_sock *req,
+static int tcp_v6_send_synack(struct sock *sk,
+			      struct request_sock *req,
 			      struct request_values *rvp,
 			      u16 queue_mapping)
 {
@@ -1057,6 +1058,7 @@ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
 	struct tcp_sock *tp = tcp_sk(sk);
 	__u32 isn = TCP_SKB_CB(skb)->when;
 	struct dst_entry *dst = NULL;
+	struct flowi6 fl6;
 	bool want_cookie = false;
 
 	if (skb->protocol == htons(ETH_P_IP))
@@ -1176,7 +1178,7 @@ static int tcp_v6_conn_request(struct sock *sk, struct sk_buff *skb)
 		 */
 		if (tmp_opt.saw_tstamp &&
 		    tcp_death_row.sysctl_tw_recycle &&
-		    (dst = inet6_csk_route_req(sk, req)) != NULL &&
+		    (dst = inet6_csk_route_req(sk, &fl6, req)) != NULL &&
 		    (peer = rt6_get_peer((struct rt6_info *)dst)) != NULL &&
 		    ipv6_addr_equal((struct in6_addr *)peer->daddr.addr.a6,
 				    &treq->rmt_addr)) {
@@ -1246,6 +1248,7 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
 #ifdef CONFIG_TCP_MD5SIG
 	struct tcp_md5sig_key *key;
 #endif
+	struct flowi6 fl6;
 
 	if (skb->protocol == htons(ETH_P_IP)) {
 		/*
@@ -1308,7 +1311,7 @@ static struct sock * tcp_v6_syn_recv_sock(struct sock *sk, struct sk_buff *skb,
 		goto out_overflow;
 
 	if (!dst) {
-		dst = inet6_csk_route_req(sk, req);
+		dst = inet6_csk_route_req(sk, &fl6, req);
 		if (!dst)
 			goto out;
 	}
-- 
1.7.7.3

^ permalink raw reply related

* [PATCH 4/5] tcp: use inet6_csk_route_req() in tcp_v6_send_synack()
From: Neal Cardwell @ 2012-06-24  5:22 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Eric Dumazet, Tom Herbert, Neal Cardwell
In-Reply-To: <1340515324-2152-1-git-send-email-ncardwell@google.com>

With the recent change (earlier in this patch series) to set
flowi6_oif to treq->iif in inet6_csk_route_req(), the dst lookup in
these two functions is now identical, so tcp_v6_send_synack() can now
just call inet6_csk_route_req(), to reduce code duplication and keep
things closer to the IPv4 side, which is structured this way.

Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
 net/ipv6/tcp_ipv6.c |   29 ++++++-----------------------
 1 files changed, 6 insertions(+), 23 deletions(-)

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index cfeefbf..500a296 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -483,34 +483,17 @@ static int tcp_v6_send_synack(struct sock *sk,
 	struct inet6_request_sock *treq = inet6_rsk(req);
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct sk_buff * skb;
-	struct ipv6_txoptions *opt = NULL;
-	struct in6_addr * final_p, final;
+	struct ipv6_txoptions *opt = np->opt;
 	struct flowi6 fl6;
 	struct dst_entry *dst;
-	int err;
-
-	memset(&fl6, 0, sizeof(fl6));
-	fl6.flowi6_proto = IPPROTO_TCP;
-	fl6.daddr = treq->rmt_addr;
-	fl6.saddr = treq->loc_addr;
-	fl6.flowlabel = 0;
-	fl6.flowi6_oif = treq->iif;
-	fl6.flowi6_mark = sk->sk_mark;
-	fl6.fl6_dport = inet_rsk(req)->rmt_port;
-	fl6.fl6_sport = inet_rsk(req)->loc_port;
-	security_req_classify_flow(req, flowi6_to_flowi(&fl6));
-
-	opt = np->opt;
-	final_p = fl6_update_dst(&fl6, opt, &final);
+	int err = -ENOMEM;
 
-	dst = ip6_dst_lookup_flow(sk, &fl6, final_p, false);
-	if (IS_ERR(dst)) {
-		err = PTR_ERR(dst);
-		dst = NULL;
+	dst = inet6_csk_route_req(sk, &fl6, req);
+	if (!dst)
 		goto done;
-	}
+
 	skb = tcp_make_synack(sk, dst, req, rvp);
-	err = -ENOMEM;
+
 	if (skb) {
 		__tcp_v6_send_check(skb, &treq->loc_addr, &treq->rmt_addr);
 
-- 
1.7.7.3

^ permalink raw reply related

* [PATCH 5/5] tcp: plug dst leak in tcp_v6_conn_request()
From: Neal Cardwell @ 2012-06-24  5:22 UTC (permalink / raw)
  To: David Miller; +Cc: netdev, Eric Dumazet, Tom Herbert, Neal Cardwell
In-Reply-To: <1340515324-2152-1-git-send-email-ncardwell@google.com>

The code in tcp_v6_conn_request() was implicitly assuming that
tcp_v6_send_synack() would take care of dst_release(), much as
tcp_v4_send_synack() already does. This resulted in
tcp_v6_conn_request() leaking a dst if sysctl_tw_recycle is enabled.

This commit restructures tcp_v6_send_synack() so that it accepts a dst
pointer and takes care of releasing the dst that is passed in, to plug
the leak and avoid future surprises by bringing the IPv6 behavior in
line with the IPv4 side.

Signed-off-by: Neal Cardwell <ncardwell@google.com>
---
 net/ipv6/tcp_ipv6.c |   19 ++++++++++---------
 1 files changed, 10 insertions(+), 9 deletions(-)

diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index 500a296..846fe80 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -475,7 +475,8 @@ out:
 }
 
 
-static int tcp_v6_send_synack(struct sock *sk,
+static int tcp_v6_send_synack(struct sock *sk, struct dst_entry *dst,
+			      struct flowi6 *fl6,
 			      struct request_sock *req,
 			      struct request_values *rvp,
 			      u16 queue_mapping)
@@ -484,12 +485,10 @@ static int tcp_v6_send_synack(struct sock *sk,
 	struct ipv6_pinfo *np = inet6_sk(sk);
 	struct sk_buff * skb;
 	struct ipv6_txoptions *opt = np->opt;
-	struct flowi6 fl6;
-	struct dst_entry *dst;
 	int err = -ENOMEM;
 
-	dst = inet6_csk_route_req(sk, &fl6, req);
-	if (!dst)
+	/* First, grab a route. */
+	if (!dst && (dst = inet6_csk_route_req(sk, fl6, req)) == NULL)
 		goto done;
 
 	skb = tcp_make_synack(sk, dst, req, rvp);
@@ -497,9 +496,9 @@ static int tcp_v6_send_synack(struct sock *sk,
 	if (skb) {
 		__tcp_v6_send_check(skb, &treq->loc_addr, &treq->rmt_addr);
 
-		fl6.daddr = treq->rmt_addr;
+		fl6->daddr = treq->rmt_addr;
 		skb_set_queue_mapping(skb, queue_mapping);
-		err = ip6_xmit(sk, skb, &fl6, opt, np->tclass);
+		err = ip6_xmit(sk, skb, fl6, opt, np->tclass);
 		err = net_xmit_eval(err);
 	}
 
@@ -513,8 +512,10 @@ done:
 static int tcp_v6_rtx_synack(struct sock *sk, struct request_sock *req,
 			     struct request_values *rvp)
 {
+	struct flowi6 fl6;
+
 	TCP_INC_STATS_BH(sock_net(sk), TCP_MIB_RETRANSSEGS);
-	return tcp_v6_send_synack(sk, req, rvp, 0);
+	return tcp_v6_send_synack(sk, NULL, &fl6, req, rvp, 0);
 }
 
 static void tcp_v6_reqsk_destructor(struct request_sock *req)
@@ -1200,7 +1201,7 @@ have_isn:
 	if (security_inet_conn_request(sk, skb, req))
 		goto drop_and_release;
 
-	if (tcp_v6_send_synack(sk, req,
+	if (tcp_v6_send_synack(sk, dst, &fl6, req,
 			       (struct request_values *)&tmp_ext,
 			       skb_get_queue_mapping(skb)) ||
 	    want_cookie)
-- 
1.7.7.3

^ permalink raw reply related

* Re: [PATCH net-next] tcp: Fix bug in tcp socket early demux
From: David Miller @ 2012-06-24  6:23 UTC (permalink / raw)
  To: subramanian.vijay; +Cc: netdev, eric.dumazet, alexander.h.duyck
In-Reply-To: <1340509090-2819-1-git-send-email-subramanian.vijay@gmail.com>

From: Vijay Subramanian <subramanian.vijay@gmail.com>
Date: Sat, 23 Jun 2012 20:38:10 -0700

> The dest port for the call to __inet_lookup_established() in TCP early demux
> code is passed with the wrong endian-ness. This causes the lookup to fail
> leading to early demux not being used.
> 
> Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>

That's what I get for only testing on big-endian :-)

Applied, thanks.

^ permalink raw reply

* Re: [PATCH 0/2 net] Bug fixes for batman-adv 2012-06-23
From: David Miller @ 2012-06-24  6:24 UTC (permalink / raw)
  To: ordex; +Cc: netdev, b.a.t.m.a.n
In-Reply-To: <1340465459-2949-1-git-send-email-ordex@autistici.org>

From: Antonio Quartulli <ordex@autistici.org>
Date: Sat, 23 Jun 2012 17:30:57 +0200

> Patch 1 is a fix for the AP-Isolation feature. A wrong check made all the
> broadcast packets coming from any client be dropped before delivery to the
> interface.
> Patch 2 instead fixes a "real" race condition in the TranslationTable code.
 ...
>   git://git.open-mesh.org/linux-merge.git batman-adv/maint

Pulled, thanks.

^ permalink raw reply

* Re: [PATCH 0/2 net] Bug fixes for batman-adv 2012-06-23
From: David Miller @ 2012-06-24  6:24 UTC (permalink / raw)
  To: ordex; +Cc: netdev, b.a.t.m.a.n
In-Reply-To: <20120623154512.GA14022@ritirata.org>

From: Antonio Quartulli <ordex@autistici.org>
Date: Sat, 23 Jun 2012 17:45:12 +0200

> after pulling these patchset in net, you should hit a conflict while trying to
> merge net into net-next. The conflict is caused by the renaming patches that you
> already have in the next tree.
> 
> Here are our instructions about how to solve it. Hope they will help.

Thanks a lot for this.

^ permalink raw reply

* Re: [PATCH 5/5] tcp: plug dst leak in tcp_v6_conn_request()
From: Eric Dumazet @ 2012-06-24  6:44 UTC (permalink / raw)
  To: Neal Cardwell; +Cc: David Miller, netdev, Eric Dumazet, Tom Herbert
In-Reply-To: <1340515324-2152-5-git-send-email-ncardwell@google.com>

On Sun, 2012-06-24 at 01:22 -0400, Neal Cardwell wrote:
> The code in tcp_v6_conn_request() was implicitly assuming that
> tcp_v6_send_synack() would take care of dst_release(), much as
> tcp_v4_send_synack() already does. This resulted in
> tcp_v6_conn_request() leaking a dst if sysctl_tw_recycle is enabled.
> 
> This commit restructures tcp_v6_send_synack() so that it accepts a dst
> pointer and takes care of releasing the dst that is passed in, to plug
> the leak and avoid future surprises by bringing the IPv6 behavior in
> line with the IPv4 side.
> 
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> ---

Neal, your patches are for net-next, right ?

^ permalink raw reply

* Re: [PATCH 1/5] tcp: heed result of security_inet_conn_request() in tcp_v6_conn_request()
From: Eric Dumazet @ 2012-06-24  7:36 UTC (permalink / raw)
  To: Neal Cardwell; +Cc: David Miller, netdev, Eric Dumazet, Tom Herbert
In-Reply-To: <1340515324-2152-1-git-send-email-ncardwell@google.com>

On Sun, 2012-06-24 at 01:22 -0400, Neal Cardwell wrote:
> If security_inet_conn_request() returns non-zero then TCP/IPv6 should
> drop the request, just as in TCP/IPv4 and DCCP in both IPv4 and IPv6.
> 
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> ---
>  net/ipv6/tcp_ipv6.c |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
> 
> diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
> index 3a9aec2..9df64a5 100644
> --- a/net/ipv6/tcp_ipv6.c
> +++ b/net/ipv6/tcp_ipv6.c
> @@ -1212,7 +1212,8 @@ have_isn:
>  	tcp_rsk(req)->snt_isn = isn;
>  	tcp_rsk(req)->snt_synack = tcp_time_stamp;
>  
> -	security_inet_conn_request(sk, skb, req);
> +	if (security_inet_conn_request(sk, skb, req))
> +		goto drop_and_release;
>  
>  	if (tcp_v6_send_synack(sk, req,
>  			       (struct request_values *)&tmp_ext,

Acked-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* Re: [PATCH 2/5] tcp: fix inet6_csk_route_req() for link-local addresses
From: Eric Dumazet @ 2012-06-24  7:37 UTC (permalink / raw)
  To: Neal Cardwell; +Cc: David Miller, netdev, Eric Dumazet, Tom Herbert
In-Reply-To: <1340515324-2152-2-git-send-email-ncardwell@google.com>

On Sun, 2012-06-24 at 01:22 -0400, Neal Cardwell wrote:
> Fix inet6_csk_route_req() to use as the flowi6_oif the treq->iif,
> which is correctly fixed up in tcp_v6_conn_request() to handle the
> case of link-local addresses. This brings it in line with the
> tcp_v6_send_synack() code, which is already correctly using the
> treq->iif in this way.
> 
> Signed-off-by: Neal Cardwell <ncardwell@google.com>
> ---
>  net/ipv6/inet6_connection_sock.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/net/ipv6/inet6_connection_sock.c b/net/ipv6/inet6_connection_sock.c
> index e6cee52..e23d354 100644
> --- a/net/ipv6/inet6_connection_sock.c
> +++ b/net/ipv6/inet6_connection_sock.c
> @@ -68,7 +68,7 @@ struct dst_entry *inet6_csk_route_req(struct sock *sk,
>  	fl6.daddr = treq->rmt_addr;
>  	final_p = fl6_update_dst(&fl6, np->opt, &final);
>  	fl6.saddr = treq->loc_addr;
> -	fl6.flowi6_oif = sk->sk_bound_dev_if;
> +	fl6.flowi6_oif = treq->iif;
>  	fl6.flowi6_mark = sk->sk_mark;
>  	fl6.fl6_dport = inet_rsk(req)->rmt_port;
>  	fl6.fl6_sport = inet_rsk(req)->loc_port;

Acked-by: Eric Dumazet <edumazet@google.com>

^ permalink raw reply

* Re: [PATCH 5/5] tcp: plug dst leak in tcp_v6_conn_request()
From: Eric Dumazet @ 2012-06-24  7:41 UTC (permalink / raw)
  To: Neal Cardwell; +Cc: David Miller, netdev, Eric Dumazet, Tom Herbert
In-Reply-To: <1340515324-2152-5-git-send-email-ncardwell@google.com>

On Sun, 2012-06-24 at 01:22 -0400, Neal Cardwell wrote:
> The code in tcp_v6_conn_request() was implicitly assuming that
> tcp_v6_send_synack() would take care of dst_release(), much as
> tcp_v4_send_synack() already does. This resulted in
> tcp_v6_conn_request() leaking a dst if sysctl_tw_recycle is enabled.
> 
> This commit restructures tcp_v6_send_synack() so that it accepts a dst
> pointer and takes care of releasing the dst that is passed in, to plug
> the leak and avoid future surprises by bringing the IPv6 behavior in
> line with the IPv4 side.

I feel a bit uncomfortable to send a mix of 3 patches to fix one bug.

Could you instead send pure fix (fixing dst leak) for net tree ?

Then, when fix is incorporated in net-next, send the cleanups ?

This also clashes with this pending work, so it would ease things if you
can respin the cleanups for net-next

http://patchwork.ozlabs.org/patch/166737/

Thanks !

^ permalink raw reply

* Re: [PATCH net] net: qmi_wwan: fix Oops while disconnecting
From: Bjørn Mork @ 2012-06-24  9:34 UTC (permalink / raw)
  To: Oliver Neukum
  Cc: Ming Lei, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA, Marius Bjørnstad Kotsbak
In-Reply-To: <201206232255.08319.oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org>

Oliver Neukum <oliver-GvhC2dPhHPQdnm+yROfE0A@public.gmane.org> writes:
> Am Samstag, 23. Juni 2012, 17:32:09 schrieb Bjørn Mork:
>> Ming Lei <tom.leiming-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> writes:
>> > On Sat, Jun 23, 2012 at 4:45 PM, Bjørn Mork <bjorn-yOkvZcmFvRU@public.gmane.org> wrote:
>
>> > Suppose there will be another usbnet driver which has its own subdriver
>> > too, the same trick of checking need to be added again if not taking the
>> > general way of simply removing 'usb_set_intfdata(intf, NULL);' in
>> > usbnet_disconnect.
>> 
>> Yes, I guess so.
>> 
>> I am just worrying (maybe too much) about the unknown consequences of
>> removing that code in usbnet, not fully understanding why it was there
>> in the first place.  And I do not want to take the blame and cleanup
>> work if anything goes wrong :-) Fixing it in qmi_wwan feels much safer.
>
> void usbnet_cdc_unbind(struct usbnet *dev, struct usb_interface *intf)
> {
>         struct cdc_state                *info = (void *) &dev->data;
>         struct usb_driver               *driver = driver_of(intf);
>
>         /* disconnect master --> disconnect slave */
>         if (intf == info->control && info->data) {
>                 /* ensure immediate exit from usbnet_disconnect */
>                 usb_set_intfdata(info->data, NULL);
>                 usb_driver_release_interface(driver, info->data);
>                 info->data = NULL;
>
> 1. We mirror the minidrivers closely, which reduces errors
> 2. unbind() is called with the data anyway and after disconnect()
>     the intfdata is not valid anyway, because the interface may have been
>     reprobed.

Sorry, I did not understand what you meant we should do here.  The extra
usb_set_intfdata(, NULL) in usbnet_disconnect() won't make any
difference for that piece of code, will it?

And the USB core ensures that intfdata is set to NULL before any
reprobing, so that will never be a problem.  That's the reason why it
seems redundant setting it in usbnet_disconnect().



Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH net] net: qmi_wwan: fix Oops while disconnecting
From: Oliver Neukum @ 2012-06-24 12:13 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: Ming Lei, netdev, linux-usb, Marius Bjørnstad Kotsbak
In-Reply-To: <877guxhx44.fsf@nemi.mork.no>

Am Sonntag, 24. Juni 2012, 11:34:19 schrieb Bjørn Mork:
> Oliver Neukum <oliver@neukum.org> writes:

> > 1. We mirror the minidrivers closely, which reduces errors
> > 2. unbind() is called with the data anyway and after disconnect()
> >     the intfdata is not valid anyway, because the interface may have been
> >     reprobed.
> 
> Sorry, I did not understand what you meant we should do here.  The extra
> usb_set_intfdata(, NULL) in usbnet_disconnect() won't make any
> difference for that piece of code, will it?

The point is that if it may be set to NULL, we always want it to be set to
NULL, so we catch bugs.

> And the USB core ensures that intfdata is set to NULL before any
> reprobing, so that will never be a problem.  That's the reason why it
> seems redundant setting it in usbnet_disconnect().

The point is that if there is a problem because intfdata is set to NULL,
there is very likely a problem in form of a race condition, if intfdata
were not set to NULL in usbnet's disconnect handler.

	Regards
		Oliver

^ permalink raw reply

* Re: [PATCH 5/5] tcp: plug dst leak in tcp_v6_conn_request()
From: Neal Cardwell @ 2012-06-24 17:12 UTC (permalink / raw)
  To: Eric Dumazet; +Cc: David Miller, netdev, Eric Dumazet, Tom Herbert
In-Reply-To: <1340523678.23933.11.camel@edumazet-glaptop>

On Sun, Jun 24, 2012 at 3:41 AM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Sun, 2012-06-24 at 01:22 -0400, Neal Cardwell wrote:
>> The code in tcp_v6_conn_request() was implicitly assuming that
>> tcp_v6_send_synack() would take care of dst_release(), much as
>> tcp_v4_send_synack() already does. This resulted in
>> tcp_v6_conn_request() leaking a dst if sysctl_tw_recycle is enabled.
>>
>> This commit restructures tcp_v6_send_synack() so that it accepts a dst
>> pointer and takes care of releasing the dst that is passed in, to plug
>> the leak and avoid future surprises by bringing the IPv6 behavior in
>> line with the IPv4 side.
>
> I feel a bit uncomfortable to send a mix of 3 patches to fix one bug.
>
> Could you instead send pure fix (fixing dst leak) for net tree ?
>
> Then, when fix is incorporated in net-next, send the cleanups ?
>
> This also clashes with this pending work, so it would ease things if you
> can respin the cleanups for net-next
>
> http://patchwork.ozlabs.org/patch/166737/

Yes, the patches in this series were generated as patches against the
"net" tree (sorry for not indicating that).

The dst leak on the v6 sysctl_tw_recycle code path (patches 2-5) seems
like a pretty low priority, so I think we could simplify your plan
even a little further... How about this as a plan: we could apply the
first patch in the series (tcp: heed result of
security_inet_conn_request() in tcp_v6_conn_request()) to the net tree
now, and skip patches 2-5 for now. Once your pending synack work is in
net-next, I can respin patches 2-5 for net-next. How does that sound?

neal

^ permalink raw reply

* Re: [PATCH net] net: qmi_wwan: fix Oops while disconnecting
From: Bjørn Mork @ 2012-06-24 17:47 UTC (permalink / raw)
  To: Oliver Neukum; +Cc: Ming Lei, netdev, linux-usb, Marius  Bjørnstad Kotsbak
In-Reply-To: <201206241413.08339.oliver@neukum.org>

Oliver Neukum <oliver@neukum.org> wrote:
>Am Sonntag, 24. Juni 2012, 11:34:19 schrieb Bjørn Mork:
>
>> Sorry, I did not understand what you meant we should do here.  The
>extra
>> usb_set_intfdata(, NULL) in usbnet_disconnect() won't make any
>> difference for that piece of code, will it?
>
>The point is that if it may be set to NULL, we always want it to be set
>to
>NULL, so we catch bugs.
>
>> And the USB core ensures that intfdata is set to NULL before any
>> reprobing, so that will never be a problem.  That's the reason why it
>> seems redundant setting it in usbnet_disconnect().
>
>The point is that if there is a problem because intfdata is set to
>NULL,
>there is very likely a problem in form of a race condition, if intfdata
>were not set to NULL in usbnet's disconnect handler.

Thanks for explaining. Yes, that makes sense to me as well.

So then the original patch against qmi_wwan should go in, and we should leave usbnet as it is. Are everyone comfortable with that?


Bjørn

^ permalink raw reply

* [net 1/3] caif: Clear shutdown mask to zero at reconnect.
From: sjur.brandeland @ 2012-06-24 21:01 UTC (permalink / raw)
  To: davem; +Cc: netdev, sjurbren, Sjur Brændeland

From: Sjur Brændeland <sjur.brandeland@stericsson.com>

Clear caif sockets's shutdown mask at (re)connect.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 net/caif/caif_socket.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/net/caif/caif_socket.c b/net/caif/caif_socket.c
index fb89443..78f1cda 100644
--- a/net/caif/caif_socket.c
+++ b/net/caif/caif_socket.c
@@ -220,6 +220,7 @@ static void caif_ctrl_cb(struct cflayer *layr,
 						cfsk_hold, cfsk_put);
 		cf_sk->sk.sk_state = CAIF_CONNECTED;
 		set_tx_flow_on(cf_sk);
+		cf_sk->sk.sk_shutdown = 0;
 		cf_sk->sk.sk_state_change(&cf_sk->sk);
 		break;
 
-- 
1.7.5.4

^ permalink raw reply related

* [net 2/3] caif-hsi: Bugfix - Piggyback'ed embedded CAIF frame lost
From: sjur.brandeland @ 2012-06-24 21:01 UTC (permalink / raw)
  To: davem; +Cc: netdev, sjurbren, Per Ellefsen, Per Ellefsen,
	Sjur Brændeland
In-Reply-To: <1340571698-17892-1-git-send-email-sjur.brandeland@stericsson.com>

From: Per Ellefsen <Per.Ellefsen@stericsson.com>

When receiving a piggyback'ed descriptor containing an
embedded frame, but no payload, the embedded frame was
lost.

Signed-off-by: Per Ellefsen <per.ellefsen@stericsson.com>
Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 drivers/net/caif/caif_hsi.c |    4 ++--
 1 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/caif/caif_hsi.c b/drivers/net/caif/caif_hsi.c
index 1520814..1f52ff3 100644
--- a/drivers/net/caif/caif_hsi.c
+++ b/drivers/net/caif/caif_hsi.c
@@ -693,8 +693,6 @@ static void cfhsi_rx_done(struct cfhsi *cfhsi)
 			 */
 			memcpy(rx_buf, (u8 *)piggy_desc,
 					CFHSI_DESC_SHORT_SZ);
-			/* Mark no embedded frame here */
-			piggy_desc->offset = 0;
 			if (desc_pld_len == -EPROTO)
 				goto out_of_sync;
 		}
@@ -737,6 +735,8 @@ static void cfhsi_rx_done(struct cfhsi *cfhsi)
 			/* Extract any payload in piggyback descriptor. */
 			if (cfhsi_rx_desc(piggy_desc, cfhsi) < 0)
 				goto out_of_sync;
+			/* Mark no embedded frame after extracting it */
+			piggy_desc->offset = 0;
 		}
 	}
 
-- 
1.7.5.4

^ permalink raw reply related

* [net 3/3] caif-hsi: Add missing return in error path
From: sjur.brandeland @ 2012-06-24 21:01 UTC (permalink / raw)
  To: davem; +Cc: netdev, sjurbren, Sjur Brændeland
In-Reply-To: <1340571698-17892-1-git-send-email-sjur.brandeland@stericsson.com>

From: Sjur Brændeland <sjur.brandeland@stericsson.com>

Fix a missing return, causing access to freed memory.

Signed-off-by: Sjur Brændeland <sjur.brandeland@stericsson.com>
---
 drivers/net/caif/caif_hsi.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/net/caif/caif_hsi.c b/drivers/net/caif/caif_hsi.c
index 1f52ff3..4a27adb 100644
--- a/drivers/net/caif/caif_hsi.c
+++ b/drivers/net/caif/caif_hsi.c
@@ -1178,6 +1178,7 @@ int cfhsi_probe(struct platform_device *pdev)
 		dev_err(&ndev->dev, "%s: Registration error: %d.\n",
 			__func__, res);
 		free_netdev(ndev);
+		return -ENODEV;
 	}
 	/* Add CAIF HSI device to list. */
 	spin_lock(&cfhsi_list_lock);
-- 
1.7.5.4

^ permalink raw reply related

* [PATCH 1/1] ipheth: add support for iPad
From: rainbow @ 2012-06-24 22:37 UTC (permalink / raw)
  To: Greg Kroah-Hartman; +Cc: linux-usb, netdev, linux-kernel, rainbow

This adds support for the iPad to the ipheth driver.
(product id = 0x129a)

Signed-off-by: rainbow <rainbow@irh.it>
---
 drivers/net/usb/ipheth.c |    5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/net/usb/ipheth.c b/drivers/net/usb/ipheth.c
index 964031e..9c98449 100644
--- a/drivers/net/usb/ipheth.c
+++ b/drivers/net/usb/ipheth.c
@@ -59,6 +59,7 @@
 #define USB_PRODUCT_IPHONE_3G   0x1292
 #define USB_PRODUCT_IPHONE_3GS  0x1294
 #define USB_PRODUCT_IPHONE_4	0x1297
+#define USB_PRODUCT_IPAD 0x129a
 #define USB_PRODUCT_IPHONE_4_VZW 0x129c
 #define USB_PRODUCT_IPHONE_4S	0x12a0
 
@@ -101,6 +102,10 @@ static struct usb_device_id ipheth_table[] = {
 		IPHETH_USBINTF_CLASS, IPHETH_USBINTF_SUBCLASS,
 		IPHETH_USBINTF_PROTO) },
 	{ USB_DEVICE_AND_INTERFACE_INFO(
+		USB_VENDOR_APPLE, USB_PRODUCT_IPAD,
+		IPHETH_USBINTF_CLASS, IPHETH_USBINTF_SUBCLASS,
+		IPHETH_USBINTF_PROTO) },
+	{ USB_DEVICE_AND_INTERFACE_INFO(
 		USB_VENDOR_APPLE, USB_PRODUCT_IPHONE_4_VZW,
 		IPHETH_USBINTF_CLASS, IPHETH_USBINTF_SUBCLASS,
 		IPHETH_USBINTF_PROTO) },
-- 
1.7.10.2

^ permalink raw reply related

* [PATCH net-next] net: Remove 'unlikely' qualifier in skb_steal_sock()
From: Vijay Subramanian @ 2012-06-24 23:03 UTC (permalink / raw)
  To: netdev
  Cc: davem, eric.dumazet, alexander.h.duyck, shemminger,
	Vijay Subramanian

With early demux enabled by default for TCP flows, there is high chance that
skb->sk will be non-null. 'unlikely()' was removed from __inet_lookup_skb() but
maybe it can be removed from skb_steal_sock() as well.

Note: skb_steal_sock() is also called by __inet6_lookup_skb() and
__udp4_lib_lookup_skb() but they are protected by their own 'unlikely' calls.

Signed-off-by: Vijay Subramanian <subramanian.vijay@gmail.com>
---
 include/net/sock.h |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/include/net/sock.h b/include/net/sock.h
index 87b424a..2108603 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -2154,7 +2154,7 @@ static inline void sk_change_net(struct sock *sk, struct net *net)
 
 static inline struct sock *skb_steal_sock(struct sk_buff *skb)
 {
-	if (unlikely(skb->sk)) {
+	if (skb->sk) {
 		struct sock *sk = skb->sk;
 
 		skb->destructor = NULL;
-- 
1.7.0.4

^ permalink raw reply related


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox