Netdev List
 help / color / mirror / Atom feed
* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2012-06-25  3:38 UTC (permalink / raw)
  To: David Miller, netdev
  Cc: linux-next, linux-kernel, Antonio Quartulli, Sven Eckelmann

[-- Attachment #1: Type: text/plain, Size: 1033 bytes --]

Hi all,

Today's linux-next merge of the net-next tree got a conflict in
net/batman-adv/translation-table.c between commit 8b8e4bc0391f
("batman-adv: fix race condition in TT full-table replacement") from the
net tree and commit 7d211efc5087 ("batman-adv: Prefix originator
non-static functions with batadv_") from the net-next tree.

Just context changes.  I fixed it up (see below) and can carry the fix as
necessary.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

diff --cc net/batman-adv/translation-table.c
index 2ab83d7,5180d50..0000000
--- a/net/batman-adv/translation-table.c
+++ b/net/batman-adv/translation-table.c
@@@ -141,7 -139,8 +139,7 @@@ static void tt_orig_list_entry_free_rcu
  	struct tt_orig_list_entry *orig_entry;
  
  	orig_entry = container_of(rcu, struct tt_orig_list_entry, rcu);
- 	orig_node_free_ref(orig_entry->orig_node);
 -	atomic_dec(&orig_entry->orig_node->tt_size);
+ 	batadv_orig_node_free_ref(orig_entry->orig_node);
  	kfree(orig_entry);
  }
  

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* Re: [PATCH net] net: qmi_wwan: fix Oops while disconnecting
From: Ming Lei @ 2012-06-25  3:37 UTC (permalink / raw)
  To: Bjørn Mork
  Cc: Oliver Neukum, netdev, linux-usb, Marius Bjørnstad Kotsbak
In-Reply-To: <0e34226c-8fe7-4e2e-8fc2-2ed140f23e9b@email.android.com>

On Mon, Jun 25, 2012 at 1:47 AM, Bjørn Mork <bjorn@mork.no> wrote:
> 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.

I don't agree on the assumption.

The current problem is caused by the set to NULL without any
protection or sync mechanism on it, and it is really a bug.

Also we didn't say the set to NULL will be cancelled, just delay
the clear until it is safe to do it, eg. after complete of unregister_netdev()
and driver_info->unbind, when the previous .open/.close has been
completed already or the later ones will notice the disconnection
early and won't touch usb_get_intfdata() any more.


> 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?

I don't see any races caused by just removing usb_set_intfdata(, NULL)
or moving it after driver_info->unbind simply in usbnet_disconnect.

In fact, suppose that .open/.close and .disconnect are run on different CPUs,
there are no any guarantee that .open/.close can always see the clear action
immediately. Also, the clear of intfdata may not be observed in .manage_power
since usb_set_intfdata(, NULL) may be completed after the lock wdm_mutex
operation.

So it is only the sync mechanism that  works on the race even the check is
added in the patch.  Putting usb_set_intfdata(, NULL) after driver_info->unbind
should be OK, and it is a general solution for the problem.


Thanks,
-- 
Ming Lei

^ permalink raw reply

* linux-next: manual merge of the net-next tree with the net tree
From: Stephen Rothwell @ 2012-06-25  3:33 UTC (permalink / raw)
  To: David Miller, netdev; +Cc: linux-next, linux-kernel, "Bjørn Mork"

[-- Attachment #1: Type: text/plain, Size: 500 bytes --]

Hi all,

Today's linux-next merge of the net-next tree got a conflict in
drivers/net/usb/qmi_wwan.c between commit b9f90eb27402 ("net: qmi_wwan:
fix Gobi device probing") from the net tree and various commits from the
net-next tree.

I am not sure how to fix this, but the comments in the net tree commit
implied that it would be placed in the 3.6 code, so I just used the
version of this file from the net-next tree.
-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au

[-- Attachment #2: Type: application/pgp-signature, Size: 836 bytes --]

^ permalink raw reply

* RE: [PATCH] r8169: RxConfig hack for the 8168evl.
From: hayeswang @ 2012-06-25  3:31 UTC (permalink / raw)
  To: 'Francois Romieu'; +Cc: netdev, thomas.pi
In-Reply-To: <20120620220918.GA2785@electric-eye.fr.zoreil.com>

Hi, 

> - the patch sets the RX_MULTI_EN bit. If the 8168c doc is any guide,
>   the chipset now fetches several Rx descriptors at a time.
> - long ago the driver ignored the RX_MULTI_EN bit.
>   e542a2269f232d61270ceddd42b73a4348dee2bb changed the RxConfig
>   settings. Whatever the problem it's now labeled a regression.

The definition of the IO 0x44 bit 14 is opposite for new chips.
For 8111C, 0 means fetching one Rx descriptor, and 1 means fetching
multi-descriptors.
For 8111D and the later chips, 0 means fetching multi-descriptors, and 1 means
fetching one Rx descriptor.

However, I have no idea about why it influences the issue.

> - Realtek's own driver can identify two different 8168evl devices
>   (CFG_METHOD_16 and CFG_METHOD_17) where the r8169 driver only
>   sees one. It sucks.

The CFG_METHOD_16 is the internal test chip. We don't have mass production for
it. Even it could be removed from driver. I don't think the kernel have to
support it.
 
Best Regards,
Hayes

^ permalink raw reply

* Re: [PATCH 1/1] ipheth: add support for iPad
From: David Miller @ 2012-06-25  2:53 UTC (permalink / raw)
  To: jj; +Cc: rainbow, gregkh, linux-usb, netdev, linux-kernel
In-Reply-To: <alpine.LNX.2.00.1206250208140.30361@swampdragon.chaosbits.net>

From: Jesper Juhl <jj@chaosbits.net>
Date: Mon, 25 Jun 2012 02:09:22 +0200 (CEST)

> On Mon, 25 Jun 2012, rainbow wrote:
> 
>> This adds support for the iPad to the ipheth driver.
>> (product id = 0x129a)
>> 
>> Signed-off-by: rainbow <rainbow@irh.it>
> 
> It is usually very much prefered that people use their real names in 
> Signed-off-by: lines.
> 
> Please consider re-submitting using your real name.

Indeed.

^ permalink raw reply

* Re: [PATCH 1/1] ipheth: add support for iPad
From: Jesper Juhl @ 2012-06-25  0:09 UTC (permalink / raw)
  To: rainbow; +Cc: Greg Kroah-Hartman, linux-usb, netdev, linux-kernel
In-Reply-To: <1340577468-3298-1-git-send-email-rainbow@irh.it>

On Mon, 25 Jun 2012, rainbow wrote:

> This adds support for the iPad to the ipheth driver.
> (product id = 0x129a)
> 
> Signed-off-by: rainbow <rainbow@irh.it>

It is usually very much prefered that people use their real names in 
Signed-off-by: lines.

Please consider re-submitting using your real name.

-- 
Jesper Juhl <jj@chaosbits.net>       http://www.chaosbits.net/
Don't top-post http://www.catb.org/jargon/html/T/top-post.html
Plain text mails only, please.

^ permalink raw reply

* [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

* [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

* [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

* [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 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

* 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

* 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: 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 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 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 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 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 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 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 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 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

* [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

* [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 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


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