[PATCH] sctp: fix warning at inet_sock_destruct() while release sctp

[PATCH] sctp: fix warning at inet_sock_destruct() while release sctp

[Wei Yongjun]

Commit 'net: Move rx skb_orphan call to where needed' broken sctp protocol
with warning at inet_sock_destruct(). Actually, sctp can do this right with
sctp_sock_rfree_frag() and sctp_skb_set_owner_r_frag() pair.

sctp_sock_rfree_frag(skb);
sctp_skb_set_owner_r_frag(skb, newsk);

This patch not revert the commit d55d87fdff8252d0e2f7c28c2d443aee17e9d70f,
instead remove the sctp_sock_rfree_frag() function.

------------[ cut here ]------------
WARNING: at net/ipv4/af_inet.c:151 inet_sock_destruct+0xe0/0x142()
Modules linked in: sctp ipv6 dm_mirror dm_region_hash dm_log dm_multipath
scsi_mod ext3 jbd uhci_hcd ohci_hcd ehci_hcd [last unloaded: scsi_wait_scan]
Pid: 1808, comm: sctp_test Not tainted 2.6.31-rc2 #40
Call Trace:
[<c042dd06>] warn_slowpath_common+0x6a/0x81
[<c064a39a>] ? inet_sock_destruct+0xe0/0x142
[<c042dd2f>] warn_slowpath_null+0x12/0x15
[<c064a39a>] inet_sock_destruct+0xe0/0x142
[<c05fde44>] __sk_free+0x19/0xcc
[<c05fdf50>] sk_free+0x18/0x1a
[<ca0d14ad>] sctp_close+0x192/0x1a1 [sctp]
[<c0649f7f>] inet_release+0x47/0x4d
[<c05fba4d>] sock_release+0x19/0x5e
[<c05fbab3>] sock_close+0x21/0x25
[<c049c31b>] __fput+0xde/0x189
[<c049c3de>] fput+0x18/0x1a
[<c049988f>] filp_close+0x56/0x60
[<c042f422>] put_files_struct+0x5d/0xa1
[<c042f49f>] exit_files+0x39/0x3d
[<c043086a>] do_exit+0x1a5/0x5dd
[<c04a86c2>] ? d_kill+0x35/0x3b
[<c0438fa4>] ? dequeue_signal+0xa6/0x115
[<c0430d05>] do_group_exit+0x63/0x8a
[<c0439504>] get_signal_to_deliver+0x2e1/0x2f9
[<c0401d9e>] do_notify_resume+0x7c/0x6b5
[<c043f601>] ? autoremove_wake_function+0x0/0x34
[<c04a864e>] ? __d_free+0x3d/0x40
[<c04a867b>] ? d_free+0x2a/0x3c
[<c049ba7e>] ? vfs_write+0x103/0x117
[<c05fc8fa>] ? sys_socketcall+0x178/0x182
[<c0402a56>] work_notifysig+0x13/0x19
---[ end trace 9db92c463e789fba ]---

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
net/sctp/socket.c | 25 ++-----------------------
1 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 35ba035..971890d 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -6652,21 +6652,6 @@ static void sctp_wait_for_close(struct sock *sk,
long timeout)
finish_wait(sk->sk_sleep, &wait);
}

-static void sctp_sock_rfree_frag(struct sk_buff *skb)
-{
- struct sk_buff *frag;
-
- if (!skb->data_len)
- goto done;
-
- /* Don't forget the fragments. */
- skb_walk_frags(skb, frag)
- sctp_sock_rfree_frag(frag);
-
-done:
- sctp_sock_rfree(skb);
-}
-
static void sctp_skb_set_owner_r_frag(struct sk_buff *skb, struct sock *sk)
{
struct sk_buff *frag;
@@ -6776,7 +6761,6 @@ static void sctp_sock_migrate(struct sock *oldsk,
struct sock *newsk,
sctp_skb_for_each(skb, &oldsk->sk_receive_queue, tmp) {
event = sctp_skb2event(skb);
if (event->asoc = assoc) {
- sctp_sock_rfree_frag(skb);
__skb_unlink(skb, &oldsk->sk_receive_queue);
__skb_queue_tail(&newsk->sk_receive_queue, skb);
sctp_skb_set_owner_r_frag(skb, newsk);
@@ -6807,7 +6791,6 @@ static void sctp_sock_migrate(struct sock *oldsk,
struct sock *newsk,
sctp_skb_for_each(skb, &oldsp->pd_lobby, tmp) {
event = sctp_skb2event(skb);
if (event->asoc = assoc) {
- sctp_sock_rfree_frag(skb);
__skb_unlink(skb, &oldsp->pd_lobby);
__skb_queue_tail(queue, skb);
sctp_skb_set_owner_r_frag(skb, newsk);
@@ -6822,15 +6805,11 @@ static void sctp_sock_migrate(struct sock
*oldsk, struct sock *newsk,

}

- sctp_skb_for_each(skb, &assoc->ulpq.reasm, tmp) {
- sctp_sock_rfree_frag(skb);
+ sctp_skb_for_each(skb, &assoc->ulpq.reasm, tmp)
sctp_skb_set_owner_r_frag(skb, newsk);
- }

- sctp_skb_for_each(skb, &assoc->ulpq.lobby, tmp) {
- sctp_sock_rfree_frag(skb);
+ sctp_skb_for_each(skb, &assoc->ulpq.lobby, tmp)
sctp_skb_set_owner_r_frag(skb, newsk);
- }

/* Set the type of socket to indicate that it is peeled off from the
* original UDP-style socket or created with the accept() call on a
-- 
1.6.2.2

[PATCH] sctp: fix warning at inet_sock_destruct() while release sctp socket

[Wei Yongjun]

Commit 'net: Move rx skb_orphan call to where needed' broken sctp protocol
with warning at inet_sock_destruct(). Actually, sctp can do this right with
sctp_sock_rfree_frag() and sctp_skb_set_owner_r_frag() pair.

sctp_sock_rfree_frag(skb);
sctp_skb_set_owner_r_frag(skb, newsk);

This patch not revert the commit d55d87fdff8252d0e2f7c28c2d443aee17e9d70f,
instead remove the sctp_sock_rfree_frag() function.

------------[ cut here ]------------
WARNING: at net/ipv4/af_inet.c:151 inet_sock_destruct+0xe0/0x142()
Modules linked in: sctp ipv6 dm_mirror dm_region_hash dm_log dm_multipath
scsi_mod ext3 jbd uhci_hcd ohci_hcd ehci_hcd [last unloaded: scsi_wait_scan]
Pid: 1808, comm: sctp_test Not tainted 2.6.31-rc2 #40
Call Trace:
[<c042dd06>] warn_slowpath_common+0x6a/0x81
[<c064a39a>] ? inet_sock_destruct+0xe0/0x142
[<c042dd2f>] warn_slowpath_null+0x12/0x15
[<c064a39a>] inet_sock_destruct+0xe0/0x142
[<c05fde44>] __sk_free+0x19/0xcc
[<c05fdf50>] sk_free+0x18/0x1a
[<ca0d14ad>] sctp_close+0x192/0x1a1 [sctp]
[<c0649f7f>] inet_release+0x47/0x4d
[<c05fba4d>] sock_release+0x19/0x5e
[<c05fbab3>] sock_close+0x21/0x25
[<c049c31b>] __fput+0xde/0x189
[<c049c3de>] fput+0x18/0x1a
[<c049988f>] filp_close+0x56/0x60
[<c042f422>] put_files_struct+0x5d/0xa1
[<c042f49f>] exit_files+0x39/0x3d
[<c043086a>] do_exit+0x1a5/0x5dd
[<c04a86c2>] ? d_kill+0x35/0x3b
[<c0438fa4>] ? dequeue_signal+0xa6/0x115
[<c0430d05>] do_group_exit+0x63/0x8a
[<c0439504>] get_signal_to_deliver+0x2e1/0x2f9
[<c0401d9e>] do_notify_resume+0x7c/0x6b5
[<c043f601>] ? autoremove_wake_function+0x0/0x34
[<c04a864e>] ? __d_free+0x3d/0x40
[<c04a867b>] ? d_free+0x2a/0x3c
[<c049ba7e>] ? vfs_write+0x103/0x117
[<c05fc8fa>] ? sys_socketcall+0x178/0x182
[<c0402a56>] work_notifysig+0x13/0x19
---[ end trace 9db92c463e789fba ]---

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
net/sctp/socket.c | 25 ++-----------------------
1 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 35ba035..971890d 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -6652,21 +6652,6 @@ static void sctp_wait_for_close(struct sock *sk,
long timeout)
finish_wait(sk->sk_sleep, &wait);
}

-static void sctp_sock_rfree_frag(struct sk_buff *skb)
-{
- struct sk_buff *frag;
-
- if (!skb->data_len)
- goto done;
-
- /* Don't forget the fragments. */
- skb_walk_frags(skb, frag)
- sctp_sock_rfree_frag(frag);
-
-done:
- sctp_sock_rfree(skb);
-}
-
static void sctp_skb_set_owner_r_frag(struct sk_buff *skb, struct sock *sk)
{
struct sk_buff *frag;
@@ -6776,7 +6761,6 @@ static void sctp_sock_migrate(struct sock *oldsk,
struct sock *newsk,
sctp_skb_for_each(skb, &oldsk->sk_receive_queue, tmp) {
event = sctp_skb2event(skb);
if (event->asoc == assoc) {
- sctp_sock_rfree_frag(skb);
__skb_unlink(skb, &oldsk->sk_receive_queue);
__skb_queue_tail(&newsk->sk_receive_queue, skb);
sctp_skb_set_owner_r_frag(skb, newsk);
@@ -6807,7 +6791,6 @@ static void sctp_sock_migrate(struct sock *oldsk,
struct sock *newsk,
sctp_skb_for_each(skb, &oldsp->pd_lobby, tmp) {
event = sctp_skb2event(skb);
if (event->asoc == assoc) {
- sctp_sock_rfree_frag(skb);
__skb_unlink(skb, &oldsp->pd_lobby);
__skb_queue_tail(queue, skb);
sctp_skb_set_owner_r_frag(skb, newsk);
@@ -6822,15 +6805,11 @@ static void sctp_sock_migrate(struct sock
*oldsk, struct sock *newsk,

}

- sctp_skb_for_each(skb, &assoc->ulpq.reasm, tmp) {
- sctp_sock_rfree_frag(skb);
+ sctp_skb_for_each(skb, &assoc->ulpq.reasm, tmp)
sctp_skb_set_owner_r_frag(skb, newsk);
- }

- sctp_skb_for_each(skb, &assoc->ulpq.lobby, tmp) {
- sctp_sock_rfree_frag(skb);
+ sctp_skb_for_each(skb, &assoc->ulpq.lobby, tmp)
sctp_skb_set_owner_r_frag(skb, newsk);
- }

/* Set the type of socket to indicate that it is peeled off from the
* original UDP-style socket or created with the accept() call on a
-- 
1.6.2.2

[PATCHv2] sctp: fix warning at inet_sock_destruct() while release

[Wei Yongjun]

Commit 'net: Move rx skb_orphan call to where needed' broken sctp protocol
with warning at inet_sock_destruct(). Actually, sctp can do this right with
sctp_sock_rfree_frag() and sctp_skb_set_owner_r_frag() pair.

    sctp_sock_rfree_frag(skb);
    sctp_skb_set_owner_r_frag(skb, newsk);

This patch not revert the commit d55d87fdff8252d0e2f7c28c2d443aee17e9d70f,
instead remove the sctp_sock_rfree_frag() function.

------------[ cut here ]------------
WARNING: at net/ipv4/af_inet.c:151 inet_sock_destruct+0xe0/0x142()
Modules linked in: sctp ipv6 dm_mirror dm_region_hash dm_log dm_multipath
scsi_mod ext3 jbd uhci_hcd ohci_hcd ehci_hcd [last unloaded: scsi_wait_scan]
Pid: 1808, comm: sctp_test Not tainted 2.6.31-rc2 #40
Call Trace:
 [<c042dd06>] warn_slowpath_common+0x6a/0x81
 [<c064a39a>] ? inet_sock_destruct+0xe0/0x142
 [<c042dd2f>] warn_slowpath_null+0x12/0x15
 [<c064a39a>] inet_sock_destruct+0xe0/0x142
 [<c05fde44>] __sk_free+0x19/0xcc
 [<c05fdf50>] sk_free+0x18/0x1a
 [<ca0d14ad>] sctp_close+0x192/0x1a1 [sctp]
 [<c0649f7f>] inet_release+0x47/0x4d
 [<c05fba4d>] sock_release+0x19/0x5e
 [<c05fbab3>] sock_close+0x21/0x25
 [<c049c31b>] __fput+0xde/0x189
 [<c049c3de>] fput+0x18/0x1a
 [<c049988f>] filp_close+0x56/0x60
 [<c042f422>] put_files_struct+0x5d/0xa1
 [<c042f49f>] exit_files+0x39/0x3d
 [<c043086a>] do_exit+0x1a5/0x5dd
 [<c04a86c2>] ? d_kill+0x35/0x3b
 [<c0438fa4>] ? dequeue_signal+0xa6/0x115
 [<c0430d05>] do_group_exit+0x63/0x8a
 [<c0439504>] get_signal_to_deliver+0x2e1/0x2f9
 [<c0401d9e>] do_notify_resume+0x7c/0x6b5
 [<c043f601>] ? autoremove_wake_function+0x0/0x34
 [<c04a864e>] ? __d_free+0x3d/0x40
 [<c04a867b>] ? d_free+0x2a/0x3c
 [<c049ba7e>] ? vfs_write+0x103/0x117
 [<c05fc8fa>] ? sys_socketcall+0x178/0x182
 [<c0402a56>] work_notifysig+0x13/0x19
---[ end trace 9db92c463e789fba ]---

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
 net/sctp/socket.c |   25 ++-----------------------
 1 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 35ba035..971890d 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -6652,21 +6652,6 @@ static void sctp_wait_for_close(struct sock *sk, long timeout)
 	finish_wait(sk->sk_sleep, &wait);
 }
 
-static void sctp_sock_rfree_frag(struct sk_buff *skb)
-{
-	struct sk_buff *frag;
-
-	if (!skb->data_len)
-		goto done;
-
-	/* Don't forget the fragments. */
-	skb_walk_frags(skb, frag)
-		sctp_sock_rfree_frag(frag);
-
-done:
-	sctp_sock_rfree(skb);
-}
-
 static void sctp_skb_set_owner_r_frag(struct sk_buff *skb, struct sock *sk)
 {
 	struct sk_buff *frag;
@@ -6776,7 +6761,6 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
 	sctp_skb_for_each(skb, &oldsk->sk_receive_queue, tmp) {
 		event = sctp_skb2event(skb);
 		if (event->asoc = assoc) {
-			sctp_sock_rfree_frag(skb);
 			__skb_unlink(skb, &oldsk->sk_receive_queue);
 			__skb_queue_tail(&newsk->sk_receive_queue, skb);
 			sctp_skb_set_owner_r_frag(skb, newsk);
@@ -6807,7 +6791,6 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
 		sctp_skb_for_each(skb, &oldsp->pd_lobby, tmp) {
 			event = sctp_skb2event(skb);
 			if (event->asoc = assoc) {
-				sctp_sock_rfree_frag(skb);
 				__skb_unlink(skb, &oldsp->pd_lobby);
 				__skb_queue_tail(queue, skb);
 				sctp_skb_set_owner_r_frag(skb, newsk);
@@ -6822,15 +6805,11 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
 
 	}
 
-	sctp_skb_for_each(skb, &assoc->ulpq.reasm, tmp) {
-		sctp_sock_rfree_frag(skb);
+	sctp_skb_for_each(skb, &assoc->ulpq.reasm, tmp)
 		sctp_skb_set_owner_r_frag(skb, newsk);
-	}
 
-	sctp_skb_for_each(skb, &assoc->ulpq.lobby, tmp) {
-		sctp_sock_rfree_frag(skb);
+	sctp_skb_for_each(skb, &assoc->ulpq.lobby, tmp)
 		sctp_skb_set_owner_r_frag(skb, newsk);
-	}
 
 	/* Set the type of socket to indicate that it is peeled off from the
 	 * original UDP-style socket or created with the accept() call on a
-- 
1.6.2.2

[PATCHv2] sctp: fix warning at inet_sock_destruct() while release sctp socket

[Wei Yongjun]

Commit 'net: Move rx skb_orphan call to where needed' broken sctp protocol
with warning at inet_sock_destruct(). Actually, sctp can do this right with
sctp_sock_rfree_frag() and sctp_skb_set_owner_r_frag() pair.

    sctp_sock_rfree_frag(skb);
    sctp_skb_set_owner_r_frag(skb, newsk);

This patch not revert the commit d55d87fdff8252d0e2f7c28c2d443aee17e9d70f,
instead remove the sctp_sock_rfree_frag() function.

------------[ cut here ]------------
WARNING: at net/ipv4/af_inet.c:151 inet_sock_destruct+0xe0/0x142()
Modules linked in: sctp ipv6 dm_mirror dm_region_hash dm_log dm_multipath
scsi_mod ext3 jbd uhci_hcd ohci_hcd ehci_hcd [last unloaded: scsi_wait_scan]
Pid: 1808, comm: sctp_test Not tainted 2.6.31-rc2 #40
Call Trace:
 [<c042dd06>] warn_slowpath_common+0x6a/0x81
 [<c064a39a>] ? inet_sock_destruct+0xe0/0x142
 [<c042dd2f>] warn_slowpath_null+0x12/0x15
 [<c064a39a>] inet_sock_destruct+0xe0/0x142
 [<c05fde44>] __sk_free+0x19/0xcc
 [<c05fdf50>] sk_free+0x18/0x1a
 [<ca0d14ad>] sctp_close+0x192/0x1a1 [sctp]
 [<c0649f7f>] inet_release+0x47/0x4d
 [<c05fba4d>] sock_release+0x19/0x5e
 [<c05fbab3>] sock_close+0x21/0x25
 [<c049c31b>] __fput+0xde/0x189
 [<c049c3de>] fput+0x18/0x1a
 [<c049988f>] filp_close+0x56/0x60
 [<c042f422>] put_files_struct+0x5d/0xa1
 [<c042f49f>] exit_files+0x39/0x3d
 [<c043086a>] do_exit+0x1a5/0x5dd
 [<c04a86c2>] ? d_kill+0x35/0x3b
 [<c0438fa4>] ? dequeue_signal+0xa6/0x115
 [<c0430d05>] do_group_exit+0x63/0x8a
 [<c0439504>] get_signal_to_deliver+0x2e1/0x2f9
 [<c0401d9e>] do_notify_resume+0x7c/0x6b5
 [<c043f601>] ? autoremove_wake_function+0x0/0x34
 [<c04a864e>] ? __d_free+0x3d/0x40
 [<c04a867b>] ? d_free+0x2a/0x3c
 [<c049ba7e>] ? vfs_write+0x103/0x117
 [<c05fc8fa>] ? sys_socketcall+0x178/0x182
 [<c0402a56>] work_notifysig+0x13/0x19
---[ end trace 9db92c463e789fba ]---

Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
---
 net/sctp/socket.c |   25 ++-----------------------
 1 files changed, 2 insertions(+), 23 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index 35ba035..971890d 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -6652,21 +6652,6 @@ static void sctp_wait_for_close(struct sock *sk, long timeout)
 	finish_wait(sk->sk_sleep, &wait);
 }
 
-static void sctp_sock_rfree_frag(struct sk_buff *skb)
-{
-	struct sk_buff *frag;
-
-	if (!skb->data_len)
-		goto done;
-
-	/* Don't forget the fragments. */
-	skb_walk_frags(skb, frag)
-		sctp_sock_rfree_frag(frag);
-
-done:
-	sctp_sock_rfree(skb);
-}
-
 static void sctp_skb_set_owner_r_frag(struct sk_buff *skb, struct sock *sk)
 {
 	struct sk_buff *frag;
@@ -6776,7 +6761,6 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
 	sctp_skb_for_each(skb, &oldsk->sk_receive_queue, tmp) {
 		event = sctp_skb2event(skb);
 		if (event->asoc == assoc) {
-			sctp_sock_rfree_frag(skb);
 			__skb_unlink(skb, &oldsk->sk_receive_queue);
 			__skb_queue_tail(&newsk->sk_receive_queue, skb);
 			sctp_skb_set_owner_r_frag(skb, newsk);
@@ -6807,7 +6791,6 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
 		sctp_skb_for_each(skb, &oldsp->pd_lobby, tmp) {
 			event = sctp_skb2event(skb);
 			if (event->asoc == assoc) {
-				sctp_sock_rfree_frag(skb);
 				__skb_unlink(skb, &oldsp->pd_lobby);
 				__skb_queue_tail(queue, skb);
 				sctp_skb_set_owner_r_frag(skb, newsk);
@@ -6822,15 +6805,11 @@ static void sctp_sock_migrate(struct sock *oldsk, struct sock *newsk,
 
 	}
 
-	sctp_skb_for_each(skb, &assoc->ulpq.reasm, tmp) {
-		sctp_sock_rfree_frag(skb);
+	sctp_skb_for_each(skb, &assoc->ulpq.reasm, tmp)
 		sctp_skb_set_owner_r_frag(skb, newsk);
-	}
 
-	sctp_skb_for_each(skb, &assoc->ulpq.lobby, tmp) {
-		sctp_sock_rfree_frag(skb);
+	sctp_skb_for_each(skb, &assoc->ulpq.lobby, tmp)
 		sctp_skb_set_owner_r_frag(skb, newsk);
-	}
 
 	/* Set the type of socket to indicate that it is peeled off from the
 	 * original UDP-style socket or created with the accept() call on a
-- 
1.6.2.2

Re: [PATCHv2] sctp: fix warning at inet_sock_destruct() while

[Herbert Xu]

On Mon, Jul 06, 2009 at 01:45:48PM +0800, Wei Yongjun wrote:
> Commit 'net: Move rx skb_orphan call to where needed' broken sctp protocol
> with warning at inet_sock_destruct(). Actually, sctp can do this right with
> sctp_sock_rfree_frag() and sctp_skb_set_owner_r_frag() pair.
> 
>     sctp_sock_rfree_frag(skb);
>     sctp_skb_set_owner_r_frag(skb, newsk);
> 
> This patch not revert the commit d55d87fdff8252d0e2f7c28c2d443aee17e9d70f,
> instead remove the sctp_sock_rfree_frag() function.

It took me a while to understand your patch but yeah it does seem
to be the right fix.  So essentially we were freeing the packet
twice as my newly added skb_orphan in sctp_skb_set_owner_r is
repeating what sctp_sock_rfree_frag has already done for the
migration case.

While your patch simply eliminates sctp_sock_rfree_frag so that
we only free things once.
 
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [PATCHv2] sctp: fix warning at inet_sock_destruct() while release sctp socket

[Herbert Xu]

On Mon, Jul 06, 2009 at 01:45:48PM +0800, Wei Yongjun wrote:
> Commit 'net: Move rx skb_orphan call to where needed' broken sctp protocol
> with warning at inet_sock_destruct(). Actually, sctp can do this right with
> sctp_sock_rfree_frag() and sctp_skb_set_owner_r_frag() pair.
> 
>     sctp_sock_rfree_frag(skb);
>     sctp_skb_set_owner_r_frag(skb, newsk);
> 
> This patch not revert the commit d55d87fdff8252d0e2f7c28c2d443aee17e9d70f,
> instead remove the sctp_sock_rfree_frag() function.

It took me a while to understand your patch but yeah it does seem
to be the right fix.  So essentially we were freeing the packet
twice as my newly added skb_orphan in sctp_skb_set_owner_r is
repeating what sctp_sock_rfree_frag has already done for the
migration case.

While your patch simply eliminates sctp_sock_rfree_frag so that
we only free things once.
 
> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>

Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Thanks,
-- 
Visit Openswan at http://www.openswan.org/
Email: Herbert Xu ~{PmV>HI~} <herbert@gondor.apana.org.au>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt

Re: [PATCHv2] sctp: fix warning at inet_sock_destruct() while release

[Vlad Yasevich]

Herbert Xu wrote:
> On Mon, Jul 06, 2009 at 01:45:48PM +0800, Wei Yongjun wrote:
>> Commit 'net: Move rx skb_orphan call to where needed' broken sctp protocol
>> with warning at inet_sock_destruct(). Actually, sctp can do this right with
>> sctp_sock_rfree_frag() and sctp_skb_set_owner_r_frag() pair.
>>
>>     sctp_sock_rfree_frag(skb);
>>     sctp_skb_set_owner_r_frag(skb, newsk);
>>
>> This patch not revert the commit d55d87fdff8252d0e2f7c28c2d443aee17e9d70f,
>> instead remove the sctp_sock_rfree_frag() function.
> 
> It took me a while to understand your patch but yeah it does seem
> to be the right fix.  So essentially we were freeing the packet
> twice as my newly added skb_orphan in sctp_skb_set_owner_r is
> repeating what sctp_sock_rfree_frag has already done for the
> migration case.
> 
> While your patch simply eliminates sctp_sock_rfree_frag so that
> we only free things once.
>  

Yep, I agree.

>> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> 
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Acked-by: Vlad Yasevich <vladislav.yasevich@hp.com>

> 
> Thanks,

Re: [PATCHv2] sctp: fix warning at inet_sock_destruct() while release sctp socket

[Vlad Yasevich]

Herbert Xu wrote:
> On Mon, Jul 06, 2009 at 01:45:48PM +0800, Wei Yongjun wrote:
>> Commit 'net: Move rx skb_orphan call to where needed' broken sctp protocol
>> with warning at inet_sock_destruct(). Actually, sctp can do this right with
>> sctp_sock_rfree_frag() and sctp_skb_set_owner_r_frag() pair.
>>
>>     sctp_sock_rfree_frag(skb);
>>     sctp_skb_set_owner_r_frag(skb, newsk);
>>
>> This patch not revert the commit d55d87fdff8252d0e2f7c28c2d443aee17e9d70f,
>> instead remove the sctp_sock_rfree_frag() function.
> 
> It took me a while to understand your patch but yeah it does seem
> to be the right fix.  So essentially we were freeing the packet
> twice as my newly added skb_orphan in sctp_skb_set_owner_r is
> repeating what sctp_sock_rfree_frag has already done for the
> migration case.
> 
> While your patch simply eliminates sctp_sock_rfree_frag so that
> we only free things once.
>  

Yep, I agree.

>> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
> 
> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>

Acked-by: Vlad Yasevich <vladislav.yasevich@hp.com>

> 
> Thanks,

Re: [PATCHv2] sctp: fix warning at inet_sock_destruct() while

[David Miller]

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Mon, 06 Jul 2009 09:45:01 -0400

> Herbert Xu wrote:
>> On Mon, Jul 06, 2009 at 01:45:48PM +0800, Wei Yongjun wrote:
>>> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
>> 
>> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> Acked-by: Vlad Yasevich <vladislav.yasevich@hp.com>

Applied, thanks everyone.

Re: [PATCHv2] sctp: fix warning at inet_sock_destruct() while release sctp socket

[David Miller]

From: Vlad Yasevich <vladislav.yasevich@hp.com>
Date: Mon, 06 Jul 2009 09:45:01 -0400

> Herbert Xu wrote:
>> On Mon, Jul 06, 2009 at 01:45:48PM +0800, Wei Yongjun wrote:
>>> Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com>
>> 
>> Acked-by: Herbert Xu <herbert@gondor.apana.org.au>
> 
> Acked-by: Vlad Yasevich <vladislav.yasevich@hp.com>

Applied, thanks everyone.