net: sctp: sctp_set_owner_w: fix panic during skb orphaning

PATCH In review Daniel Borkmann 5 participants 11 messages Aug 7, 2013 – Aug 13, 2013

Subsystems (3)

Networking Network drivers SCTP protocol

lore

Share & tools

lore

Daniel Borkmann

author

2013-08-07 10:42 UTC #0 lore raw

PATCH 1/1 net: sctp: sctp_set_owner_w: fix panic during skb orphaning

This patch fixes the following triggered bug ...

[  553.109742] kernel BUG at include/linux/skbuff.h:1813!
[  553.109766] invalid opcode: 0000 [#1] SMP
[  553.109789] Modules linked in: sctp libcrc32c rfcomm [...]
[  553.110259]  uinput i915 i2c_algo_bit drm_kms_helper e1000e drm ptp pps_core i2c_core wmi video sunrpc
[  553.110320] CPU: 0 PID: 1636 Comm: lt-test_1_to_1_ Not tainted 3.11.0-rc3+ #2
[  553.110350] Hardware name: LENOVO 74597D6/74597D6, BIOS 6DET60WW (3.10 ) 09/17/2009
[  553.110381] task: ffff88020a01dd40 ti: ffff880204ed0000 task.ti: ffff880204ed0000
[  553.110411] RIP: 0010:[<ffffffffa0698017>]  [<ffffffffa0698017>] skb_orphan.part.9+0x4/0x6 [sctp]
[  553.110459] RSP: 0018:ffff880204ed1bb8  EFLAGS: 00010286
[  553.110483] RAX: ffff8802086f5a40 RBX: ffff880204303300 RCX: 0000000000000000
[  553.110487] RDX: ffff880204303c28 RSI: ffff8802086f5a40 RDI: ffff880202158000
[  553.110487] RBP: ffff880204ed1bb8 R08: 0000000000000000 R09: 0000000000000000
[  553.110487] R10: ffff88022f2d9a04 R11: ffff880233001600 R12: 0000000000000000
[  553.110487] R13: ffff880204303c00 R14: ffff8802293d0000 R15: ffff880202158000
[  553.110487] FS:  00007f31b31fe740(0000) GS:ffff88023bc00000(0000) knlGS:0000000000000000
[  553.110487] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  553.110487] CR2: 000000379980e3e0 CR3: 000000020d225000 CR4: 00000000000407f0
[  553.110487] Stack:
[  553.110487]  ffff880204ed1ca8 ffffffffa068d7fc 0000000000000000 0000000000000000
[  553.110487]  0000000000000000 ffff8802293d0000 ffff880202158000 ffffffff81cb7900
[  553.110487]  0000000000000000 0000400000001c68 ffff8802086f5a40 000000000000000f
[  553.110487] Call Trace:
[  553.110487]  [<ffffffffa068d7fc>] sctp_sendmsg+0x6bc/0xc80 [sctp]
[  553.110487]  [<ffffffff8128f185>] ? sock_has_perm+0x75/0x90
[  553.110487]  [<ffffffff815a3593>] inet_sendmsg+0x63/0xb0
[  553.110487]  [<ffffffff8128f2b3>] ? selinux_socket_sendmsg+0x23/0x30
[  553.110487]  [<ffffffff8151c5d6>] sock_sendmsg+0xa6/0xd0
[  553.110487]  [<ffffffff81637b05>] ? _raw_spin_unlock_bh+0x15/0x20
[  553.110487]  [<ffffffff8151cd38>] SYSC_sendto+0x128/0x180
[  553.110487]  [<ffffffff8151ce6b>] ? SYSC_connect+0xdb/0x100
[  553.110487]  [<ffffffffa0690031>] ? sctp_inet_listen+0x71/0x1f0 [sctp]
[  553.110487]  [<ffffffff8151d35e>] SyS_sendto+0xe/0x10
[  553.110487]  [<ffffffff81640202>] system_call_fastpath+0x16/0x1b
[  553.110487] Code: e0 48 c7 c7 00 22 6a a0 e8 67 a3 f0 e0 48 c7 [...]
[  553.110487] RIP  [<ffffffffa0698017>] skb_orphan.part.9+0x4/0x6 [sctp]
[  553.110487]  RSP <ffff880204ed1bb8>
[  553.121578] ---[ end trace 46c20c5903ef5be2 ]---

... that is triggered after commit 376c7311b ("net: add a temporary sanity check in skb_orphan()"). What is happening is that we call sctp_set_owner_w() for chunks in the SCTP output path from sctp_sendmsg(). Such chunks eventually origin from constructors like sctp_make_chunk() where skb->sk = sk is being set for socket accounting. Doing a git grep -n "skb->sk" net/sctp/ shows that also in other places the socket pointer is being set, before issuing a SCTP_CMD_SEND_PKT command and the like. Since SCTP is doing it's own memory accounting anyway and has its own skb destructor functions, we should customize sctp_set_owner_w() and call skb_orphan() if we set a different owner of the skb than the current one in order to properly call their destructor function, but not run into a panic due to our non-exisiting one as we set sctp_wfree() destructor right after that. Otherwise, we can just skip orphaning and reassignment to the very same socket and only set the destructor handler.

Signed-off-by Daniel Borkmann <[email protected]>

---
Only in net-next.

 net/sctp/socket.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

net/sctp/socket.c +11 -5

index d5d5882..7459b6c 100644
@@ -153,13 +153,18 @@ static inline void sctp_set_owner_w(struct sctp_chunk *chunk)
 {
 	struct sctp_association *asoc = chunk->asoc;
 	struct sock *sk = asoc->base.sk;
+	struct sk_buff *skb = chunk->skb;
 
 	/* The sndbuf space is tracked per association.  */
 	sctp_association_hold(asoc);
 
-	skb_set_owner_w(chunk->skb, sk);
+	if (skb->sk != sk) {
+		skb_orphan(skb);
+		skb->sk = sk;
+	}
+
+	skb->destructor = sctp_wfree;
 
-	chunk->skb->destructor = sctp_wfree;
 	/* Save the chunk pointer in skb for sctp_wfree to use later.  */
 	*((struct sctp_chunk **)(chunk->skb->cb)) = chunk;

@@ -167,9 +172,10 @@ static inline void sctp_set_owner_w(struct sctp_chunk *chunk)
 				sizeof(struct sk_buff) +
 				sizeof(struct sctp_chunk);
 
-	atomic_add(sizeof(struct sctp_chunk), &sk->sk_wmem_alloc);
-	sk->sk_wmem_queued += chunk->skb->truesize;
-	sk_mem_charge(sk, chunk->skb->truesize);
+	atomic_add(sizeof(struct sctp_chunk) + skb->truesize,
+		   &sk->sk_wmem_alloc);
+	sk->sk_wmem_queued += skb->truesize;
+	sk_mem_charge(sk, skb->truesize);
 }
 
 /* Verify that this is a valid address. */

Signature · 1 line

1.7.11.7

Neil Horman

Acked-by on v1 1/1

2013-08-08 05:57 UTC #1 lore raw

Quoted 109 lines · Daniel Borkmann

This patch fixes the following triggered bug ...

[  553.109742] kernel BUG at include/linux/skbuff.h:1813!
[  553.109766] invalid opcode: 0000 [#1] SMP
[  553.109789] Modules linked in: sctp libcrc32c rfcomm [...]
[  553.110259]  uinput i915 i2c_algo_bit drm_kms_helper e1000e drm ptp pps_core i2c_core wmi video sunrpc
[  553.110320] CPU: 0 PID: 1636 Comm: lt-test_1_to_1_ Not tainted 3.11.0-rc3+ #2
[  553.110350] Hardware name: LENOVO 74597D6/74597D6, BIOS 6DET60WW (3.10 ) 09/17/2009
[  553.110381] task: ffff88020a01dd40 ti: ffff880204ed0000 task.ti: ffff880204ed0000
[  553.110411] RIP: 0010:[<ffffffffa0698017>]  [<ffffffffa0698017>] skb_orphan.part.9+0x4/0x6 [sctp]
[  553.110459] RSP: 0018:ffff880204ed1bb8  EFLAGS: 00010286
[  553.110483] RAX: ffff8802086f5a40 RBX: ffff880204303300 RCX: 0000000000000000
[  553.110487] RDX: ffff880204303c28 RSI: ffff8802086f5a40 RDI: ffff880202158000
[  553.110487] RBP: ffff880204ed1bb8 R08: 0000000000000000 R09: 0000000000000000
[  553.110487] R10: ffff88022f2d9a04 R11: ffff880233001600 R12: 0000000000000000
[  553.110487] R13: ffff880204303c00 R14: ffff8802293d0000 R15: ffff880202158000
[  553.110487] FS:  00007f31b31fe740(0000) GS:ffff88023bc00000(0000) knlGS:0000000000000000
[  553.110487] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  553.110487] CR2: 000000379980e3e0 CR3: 000000020d225000 CR4: 00000000000407f0
[  553.110487] Stack:
[  553.110487]  ffff880204ed1ca8 ffffffffa068d7fc 0000000000000000 0000000000000000
[  553.110487]  0000000000000000 ffff8802293d0000 ffff880202158000 ffffffff81cb7900
[  553.110487]  0000000000000000 0000400000001c68 ffff8802086f5a40 000000000000000f
[  553.110487] Call Trace:
[  553.110487]  [<ffffffffa068d7fc>] sctp_sendmsg+0x6bc/0xc80 [sctp]
[  553.110487]  [<ffffffff8128f185>] ? sock_has_perm+0x75/0x90
[  553.110487]  [<ffffffff815a3593>] inet_sendmsg+0x63/0xb0
[  553.110487]  [<ffffffff8128f2b3>] ? selinux_socket_sendmsg+0x23/0x30
[  553.110487]  [<ffffffff8151c5d6>] sock_sendmsg+0xa6/0xd0
[  553.110487]  [<ffffffff81637b05>] ? _raw_spin_unlock_bh+0x15/0x20
[  553.110487]  [<ffffffff8151cd38>] SYSC_sendto+0x128/0x180
[  553.110487]  [<ffffffff8151ce6b>] ? SYSC_connect+0xdb/0x100
[  553.110487]  [<ffffffffa0690031>] ? sctp_inet_listen+0x71/0x1f0 [sctp]
[  553.110487]  [<ffffffff8151d35e>] SyS_sendto+0xe/0x10
[  553.110487]  [<ffffffff81640202>] system_call_fastpath+0x16/0x1b
[  553.110487] Code: e0 48 c7 c7 00 22 6a a0 e8 67 a3 f0 e0 48 c7 [...]
[  553.110487] RIP  [<ffffffffa0698017>] skb_orphan.part.9+0x4/0x6 [sctp]
[  553.110487]  RSP <ffff880204ed1bb8>
[  553.121578] ---[ end trace 46c20c5903ef5be2 ]---

Signed-off-by Daniel Borkmann <[email protected]>

---
Only in net-next.

 net/sctp/socket.c | 16 +++++++++++-----
 1 file changed, 11 insertions(+), 5 deletions(-)

net/sctp/socket.c +11 -5

index d5d5882..7459b6c 100644
@@ -153,13 +153,18 @@ static inline void sctp_set_owner_w(struct sctp_chunk *chunk)
 {
 	struct sctp_association *asoc = chunk->asoc;
 	struct sock *sk = asoc->base.sk;
+	struct sk_buff *skb = chunk->skb;
 
 	/* The sndbuf space is tracked per association.  */
 	sctp_association_hold(asoc);
 
-	skb_set_owner_w(chunk->skb, sk);
+	if (skb->sk != sk) {
+		skb_orphan(skb);
+		skb->sk = sk;
+	}
+
+	skb->destructor = sctp_wfree;
 
-	chunk->skb->destructor = sctp_wfree;
 	/* Save the chunk pointer in skb for sctp_wfree to use later.  */
 	*((struct sctp_chunk **)(chunk->skb->cb)) = chunk;

@@ -167,9 +172,10 @@ static inline void sctp_set_owner_w(struct sctp_chunk *chunk)
 				sizeof(struct sk_buff) +
 				sizeof(struct sctp_chunk);
 
-	atomic_add(sizeof(struct sctp_chunk), &sk->sk_wmem_alloc);
-	sk->sk_wmem_queued += chunk->skb->truesize;
-	sk_mem_charge(sk, chunk->skb->truesize);
+	atomic_add(sizeof(struct sctp_chunk) + skb->truesize,
+		   &sk->sk_wmem_alloc);
+	sk->sk_wmem_queued += skb->truesize;
+	sk_mem_charge(sk, skb->truesize);
 }
 
 /* Verify that this is a valid address. */

Signature · 6 lines

1.7.11.7

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

Acked-by Neil Horman <[email protected]>

David Miller

maintainer

2013-08-09 20:37 UTC #2 lore raw

In reply to Daniel Borkmann

From: Daniel Borkmann <[email protected]>
Date: Wed, 7 Aug 2013 12:42:01 +0200

Quoted 1 line

This patch fixes the following triggered bug ...

...

Quoted 6 lines

... that is triggered after commit 376c7311b ("net: add a temporary sanity check in skb_orphan()"). What is happening is that we call sctp_set_owner_w() for chunks in the SCTP output path from sctp_sendmsg(). Such chunks eventually origin from constructors like sctp_make_chunk() where skb->sk = sk is being set for socket accounting. Doing a git grep -n "skb->sk" net/sctp/ shows that also in other places the socket pointer is being set, before issuing a

Quoted 8 lines

SCTP_CMD_SEND_PKT command and the like. Since SCTP is doing it's own memory accounting anyway and has its own skb destructor functions, we should customize sctp_set_owner_w() and call skb_orphan() if we set a different owner of the skb than the current one in order to properly call their destructor function, but not run into a panic due to our non-exisiting one as we set sctp_wfree() destructor right after that. Otherwise, we can just skip orphaning and reassignment to the very same socket and only set the destructor handler.

This debugging check is exactly trying to catch what SCTP is doing, setting skb->sk without also setting the destructor.

I would much rather see you reorganize and fix SCTP to behave properly rather than coding up a check which is essentially "if skb_orphan() bug won't trigger, call it" That defeats the whole purpose of the check.

Thanks.

Daniel Borkmann

author

2013-08-09 20:47 UTC #3 lore raw

Quoted 20 lines · David Miller

From: Daniel Borkmann <[email protected]>
Date: Wed, 7 Aug 2013 12:42:01 +0200
Quoted 1 line
This patch fixes the following triggered bug ...
...
Quoted 6 lines
... that is triggered after commit 376c7311b ("net: add a temporary sanity check in skb_orphan()"). What is happening is that we call sctp_set_owner_w() for chunks in the SCTP output path from sctp_sendmsg(). Such chunks eventually origin from constructors like sctp_make_chunk() where skb->sk = sk is being set for socket accounting. Doing a git grep -n "skb->sk" net/sctp/ shows that also in other places the socket pointer is being set, before issuing a
Quoted 8 lines
SCTP_CMD_SEND_PKT command and the like. Since SCTP is doing it's own memory accounting anyway and has its own skb destructor functions, we should customize sctp_set_owner_w() and call skb_orphan() if we set a different owner of the skb than the current one in order to properly call their destructor function, but not run into a panic due to our non-exisiting one as we set sctp_wfree() destructor right after that. Otherwise, we can just skip orphaning and reassignment to the very same socket and only set the destructor handler.

Quoted 6 lines

This debugging check is exactly trying to catch what SCTP is doing, setting skb->sk without also setting the destructor.
I would much rather see you reorganize and fix SCTP to behave properly rather than coding up a check which is essentially "if skb_orphan() bug won't trigger, call it" That defeats the whole purpose of the check.

Ok, will try to come up with a patch next week. Thanks.

Vlad Yasevich

2013-08-09 21:58 UTC #4 lore raw

PATCH 1/1 net: sctp: fix panic during skb_orphan()

In reply to David Miller

This patch fixes the following triggered bug ...

[  553.109742] kernel BUG at include/linux/skbuff.h:1813!
[  553.109766] invalid opcode: 0000 [#1] SMP
[  553.109789] Modules linked in: sctp libcrc32c rfcomm [...]
[  553.110259]  uinput i915 i2c_algo_bit drm_kms_helper e1000e drm ptp
pps_core i2c_core wmi video sunrpc
[  553.110320] CPU: 0 PID: 1636 Comm: lt-test_1_to_1_ Not tainted
3.11.0-rc3+ #2
[  553.110350] Hardware name: LENOVO 74597D6/74597D6, BIOS 6DET60WW
(3.10 ) 09/17/2009
[  553.110381] task: ffff88020a01dd40 ti: ffff880204ed0000 task.ti:
ffff880204ed0000
[  553.110411] RIP: 0010:[<ffffffffa0698017>]  [<ffffffffa0698017>]
skb_orphan.part.9+0x4/0x6 [sctp]
[  553.110459] RSP: 0018:ffff880204ed1bb8  EFLAGS: 00010286
[  553.110483] RAX: ffff8802086f5a40 RBX: ffff880204303300 RCX:
0000000000000000
[  553.110487] RDX: ffff880204303c28 RSI: ffff8802086f5a40 RDI:
ffff880202158000
[  553.110487] RBP: ffff880204ed1bb8 R08: 0000000000000000 R09:
0000000000000000
[  553.110487] R10: ffff88022f2d9a04 R11: ffff880233001600 R12:
0000000000000000
[  553.110487] R13: ffff880204303c00 R14: ffff8802293d0000 R15:
ffff880202158000
[  553.110487] FS:  00007f31b31fe740(0000) GS:ffff88023bc00000(0000)
knlGS:0000000000000000
[  553.110487] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  553.110487] CR2: 000000379980e3e0 CR3: 000000020d225000 CR4:
00000000000407f0
[  553.110487] Stack:
[  553.110487]  ffff880204ed1ca8 ffffffffa068d7fc 0000000000000000
0000000000000000
[  553.110487]  0000000000000000 ffff8802293d0000 ffff880202158000
ffffffff81cb7900
[  553.110487]  0000000000000000 0000400000001c68 ffff8802086f5a40
000000000000000f
[  553.110487] Call Trace:
[  553.110487]  [<ffffffffa068d7fc>] sctp_sendmsg+0x6bc/0xc80 [sctp]
[  553.110487]  [<ffffffff8128f185>] ? sock_has_perm+0x75/0x90
[  553.110487]  [<ffffffff815a3593>] inet_sendmsg+0x63/0xb0
[  553.110487]  [<ffffffff8128f2b3>] ? selinux_socket_sendmsg+0x23/0x30
[  553.110487]  [<ffffffff8151c5d6>] sock_sendmsg+0xa6/0xd0
[  553.110487]  [<ffffffff81637b05>] ? _raw_spin_unlock_bh+0x15/0x20
[  553.110487]  [<ffffffff8151cd38>] SYSC_sendto+0x128/0x180
[  553.110487]  [<ffffffff8151ce6b>] ? SYSC_connect+0xdb/0x100
[  553.110487]  [<ffffffffa0690031>] ? sctp_inet_listen+0x71/0x1f0
[sctp]
[  553.110487]  [<ffffffff8151d35e>] SyS_sendto+0xe/0x10
[  553.110487]  [<ffffffff81640202>] system_call_fastpath+0x16/0x1b
[  553.110487] Code: e0 48 c7 c7 00 22 6a a0 e8 67 a3 f0 e0 48 c7 [...]
[  553.110487] RIP  [<ffffffffa0698017>] skb_orphan.part.9+0x4/0x6
[sctp]
[  553.110487]  RSP <ffff880204ed1bb8>
[  553.121578] ---[ end trace 46c20c5903ef5be2 ]---

When a SCTP data chunk is created, skb->sk is set by sctp_make_chunk(). At a later time, sctp_sendmg() calls sctp_set_owner_w() which will attempt to orphan the skb and correctly set the skb->sk. The skb_orphan call result in the above crash since skb->sk was set the the destructor was not.

The simple solution is to not set skb->sk for DATA chunks (since it will be done later). Continue to do it for control chunks since sctp needs the sk at a later timer and control chunks are not currently accouted agains socket memory.

Signed-off-by Vlad Yasevich <[email protected]>

---
 net/sctp/sm_make_chunk.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

net/sctp/sm_make_chunk.c +6 -2

index 780a2d4..4ff7803 100644
@@ -1385,8 +1385,12 @@ static struct sctp_chunk *sctp_make_chunk(const struct sctp_association *asoc,
 	if (sctp_auth_send_cid(type, asoc))
 		retval->auth = 1;
 
-	/* Set the skb to the belonging sock for accounting.  */
-	skb->sk = sk;
+	/* Set the skb to the belonging sock for accounting.  Do not
+	 * do this for data chunks as that will be properly set
+	 * up later.
+	 */
+	if (type != SCTP_CID_DATA)
+		skb->sk = sk;
 
 	return retval;
 nodata:

Signature · 1 line

1.8.1.4

Eric Dumazet

2013-08-09 23:20 UTC #5 lore raw

Re: [PATCH] net: sctp: fix panic during skb_orphan()

Quoted 81 lines · Vlad Yasevich

This patch fixes the following triggered bug ...

[  553.109742] kernel BUG at include/linux/skbuff.h:1813!
[  553.109766] invalid opcode: 0000 [#1] SMP
[  553.109789] Modules linked in: sctp libcrc32c rfcomm [...]
[  553.110259]  uinput i915 i2c_algo_bit drm_kms_helper e1000e drm ptp
pps_core i2c_core wmi video sunrpc
[  553.110320] CPU: 0 PID: 1636 Comm: lt-test_1_to_1_ Not tainted
3.11.0-rc3+ #2
[  553.110350] Hardware name: LENOVO 74597D6/74597D6, BIOS 6DET60WW
(3.10 ) 09/17/2009
[  553.110381] task: ffff88020a01dd40 ti: ffff880204ed0000 task.ti:
ffff880204ed0000
[  553.110411] RIP: 0010:[<ffffffffa0698017>]  [<ffffffffa0698017>]
skb_orphan.part.9+0x4/0x6 [sctp]
[  553.110459] RSP: 0018:ffff880204ed1bb8  EFLAGS: 00010286
[  553.110483] RAX: ffff8802086f5a40 RBX: ffff880204303300 RCX:
0000000000000000
[  553.110487] RDX: ffff880204303c28 RSI: ffff8802086f5a40 RDI:
ffff880202158000
[  553.110487] RBP: ffff880204ed1bb8 R08: 0000000000000000 R09:
0000000000000000
[  553.110487] R10: ffff88022f2d9a04 R11: ffff880233001600 R12:
0000000000000000
[  553.110487] R13: ffff880204303c00 R14: ffff8802293d0000 R15:
ffff880202158000
[  553.110487] FS:  00007f31b31fe740(0000) GS:ffff88023bc00000(0000)
knlGS:0000000000000000
[  553.110487] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  553.110487] CR2: 000000379980e3e0 CR3: 000000020d225000 CR4:
00000000000407f0
[  553.110487] Stack:
[  553.110487]  ffff880204ed1ca8 ffffffffa068d7fc 0000000000000000
0000000000000000
[  553.110487]  0000000000000000 ffff8802293d0000 ffff880202158000
ffffffff81cb7900
[  553.110487]  0000000000000000 0000400000001c68 ffff8802086f5a40
000000000000000f
[  553.110487] Call Trace:
[  553.110487]  [<ffffffffa068d7fc>] sctp_sendmsg+0x6bc/0xc80 [sctp]
[  553.110487]  [<ffffffff8128f185>] ? sock_has_perm+0x75/0x90
[  553.110487]  [<ffffffff815a3593>] inet_sendmsg+0x63/0xb0
[  553.110487]  [<ffffffff8128f2b3>] ? selinux_socket_sendmsg+0x23/0x30
[  553.110487]  [<ffffffff8151c5d6>] sock_sendmsg+0xa6/0xd0
[  553.110487]  [<ffffffff81637b05>] ? _raw_spin_unlock_bh+0x15/0x20
[  553.110487]  [<ffffffff8151cd38>] SYSC_sendto+0x128/0x180
[  553.110487]  [<ffffffff8151ce6b>] ? SYSC_connect+0xdb/0x100
[  553.110487]  [<ffffffffa0690031>] ? sctp_inet_listen+0x71/0x1f0
[sctp]
[  553.110487]  [<ffffffff8151d35e>] SyS_sendto+0xe/0x10
[  553.110487]  [<ffffffff81640202>] system_call_fastpath+0x16/0x1b
[  553.110487] Code: e0 48 c7 c7 00 22 6a a0 e8 67 a3 f0 e0 48 c7 [...]
[  553.110487] RIP  [<ffffffffa0698017>] skb_orphan.part.9+0x4/0x6
[sctp]
[  553.110487]  RSP <ffff880204ed1bb8>
[  553.121578] ---[ end trace 46c20c5903ef5be2 ]---

Signed-off-by Vlad Yasevich <[email protected]>

---
 net/sctp/sm_make_chunk.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

net/sctp/sm_make_chunk.c +0 -1

index 780a2d4..4ff7803 100644
@@ -1385,8 +1385,12 @@ static struct sctp_chunk *sctp_make_chunk(const struct sctp_association *asoc,
 	if (sctp_auth_send_cid(type, asoc))
 		retval->auth = 1;
 
-	/* Set the skb to the belonging sock for accounting.  */

net/sctp/sm_make_chunk.c static struct sctp_chunk *sctp_make_chunk(const struct sctp_association *asoc, · line 1396

Quoted 10 lines

net/sctp/sm_make_chunk.c +6 -1

-	skb->sk = sk;
+	/* Set the skb to the belonging sock for accounting.  Do not
+	 * do this for data chunks as that will be properly set
+	 * up later.
+	 */
+	if (type != SCTP_CID_DATA)
+		skb->sk = sk;
 
 	return retval;
 nodata:

This is ugly IMHO.

Why not using a dummy destructor ?

And I guess this patch is for net-next ?

Vlad Yasevich

2013-08-10 00:39 UTC #6 lore raw

Re: [PATCH] net: sctp: fix panic during skb_orphan()

Quoted 90 lines · Eric Dumazet

Quoted 89 lines · Vlad Yasevich

This patch fixes the following triggered bug ...

[  553.109742] kernel BUG at include/linux/skbuff.h:1813!
[  553.109766] invalid opcode: 0000 [#1] SMP
[  553.109789] Modules linked in: sctp libcrc32c rfcomm [...]
[  553.110259]  uinput i915 i2c_algo_bit drm_kms_helper e1000e drm ptp
pps_core i2c_core wmi video sunrpc
[  553.110320] CPU: 0 PID: 1636 Comm: lt-test_1_to_1_ Not tainted
3.11.0-rc3+ #2
[  553.110350] Hardware name: LENOVO 74597D6/74597D6, BIOS 6DET60WW
(3.10 ) 09/17/2009
[  553.110381] task: ffff88020a01dd40 ti: ffff880204ed0000 task.ti:
ffff880204ed0000
[  553.110411] RIP: 0010:[<ffffffffa0698017>]  [<ffffffffa0698017>]
skb_orphan.part.9+0x4/0x6 [sctp]
[  553.110459] RSP: 0018:ffff880204ed1bb8  EFLAGS: 00010286
[  553.110483] RAX: ffff8802086f5a40 RBX: ffff880204303300 RCX:
0000000000000000
[  553.110487] RDX: ffff880204303c28 RSI: ffff8802086f5a40 RDI:
ffff880202158000
[  553.110487] RBP: ffff880204ed1bb8 R08: 0000000000000000 R09:
0000000000000000
[  553.110487] R10: ffff88022f2d9a04 R11: ffff880233001600 R12:
0000000000000000
[  553.110487] R13: ffff880204303c00 R14: ffff8802293d0000 R15:
ffff880202158000
[  553.110487] FS:  00007f31b31fe740(0000) GS:ffff88023bc00000(0000)
knlGS:0000000000000000
[  553.110487] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  553.110487] CR2: 000000379980e3e0 CR3: 000000020d225000 CR4:
00000000000407f0
[  553.110487] Stack:
[  553.110487]  ffff880204ed1ca8 ffffffffa068d7fc 0000000000000000
0000000000000000
[  553.110487]  0000000000000000 ffff8802293d0000 ffff880202158000
ffffffff81cb7900
[  553.110487]  0000000000000000 0000400000001c68 ffff8802086f5a40
000000000000000f
[  553.110487] Call Trace:
[  553.110487]  [<ffffffffa068d7fc>] sctp_sendmsg+0x6bc/0xc80 [sctp]
[  553.110487]  [<ffffffff8128f185>] ? sock_has_perm+0x75/0x90
[  553.110487]  [<ffffffff815a3593>] inet_sendmsg+0x63/0xb0
[  553.110487]  [<ffffffff8128f2b3>] ? selinux_socket_sendmsg+0x23/0x30
[  553.110487]  [<ffffffff8151c5d6>] sock_sendmsg+0xa6/0xd0
[  553.110487]  [<ffffffff81637b05>] ? _raw_spin_unlock_bh+0x15/0x20
[  553.110487]  [<ffffffff8151cd38>] SYSC_sendto+0x128/0x180
[  553.110487]  [<ffffffff8151ce6b>] ? SYSC_connect+0xdb/0x100
[  553.110487]  [<ffffffffa0690031>] ? sctp_inet_listen+0x71/0x1f0
[sctp]
[  553.110487]  [<ffffffff8151d35e>] SyS_sendto+0xe/0x10
[  553.110487]  [<ffffffff81640202>] system_call_fastpath+0x16/0x1b
[  553.110487] Code: e0 48 c7 c7 00 22 6a a0 e8 67 a3 f0 e0 48 c7 [...]
[  553.110487] RIP  [<ffffffffa0698017>] skb_orphan.part.9+0x4/0x6
[sctp]
[  553.110487]  RSP <ffff880204ed1bb8>
[  553.121578] ---[ end trace 46c20c5903ef5be2 ]---

Signed-off-by Vlad Yasevich <[email protected]>

---
  net/sctp/sm_make_chunk.c | 8 ++++++--
  1 file changed, 6 insertions(+), 2 deletions(-)

net/sctp/sm_make_chunk.c +6 -2

index 780a2d4..4ff7803 100644
@@ -1385,8 +1385,12 @@ static struct sctp_chunk *sctp_make_chunk(const struct sctp_association *asoc,
  	if (sctp_auth_send_cid(type, asoc))
  		retval->auth = 1;
 
-	/* Set the skb to the belonging sock for accounting.  */
-	skb->sk = sk;
+	/* Set the skb to the belonging sock for accounting.  Do not
+	 * do this for data chunks as that will be properly set
+	 * up later.
+	 */
+	if (type != SCTP_CID_DATA)
+		skb->sk = sk;

Quoted 4 lines

net/sctp/sm_make_chunk.c static struct sctp_chunk *sctp_make_chunk(const struct sctp_association *asoc, · line 1396
Quoted 2 lines
net/sctp/sm_make_chunk.c +0 -0
  	return retval;
  nodata:
This is ugly IMHO.

Yes, it is a bit ugly. The "proper" way is to do accounting for all SCTP chunks, but that is a much larger piece of work.

Quoted 2 lines

Why not using a dummy destructor ?

It would just be useless once we do proper accounting for all chunks. But, I'll send a different idea if this is just too ugly.

Quoted 3 lines

And I guess this patch is for net-next ?

Yes, sorry...

Anyway. I'll send an less ugly alternative...

-vlad

Quoted 2 lines

Eric Dumazet

2013-08-10 01:20 UTC #7 lore raw

Re: [PATCH] net: sctp: fix panic during skb_orphan()

Quoted 6 lines · Vlad Yasevich

Quoted 1 line · Eric Dumazet
Why not using a dummy destructor ?
It would just be useless once we do proper accounting for all chunks. But, I'll send a different idea if this is just too ugly.

Setting skb->sk is no accounting.

It seems you would better use skb->cb[] so that you are 100% sure this skb->sk doesn't leak outside of SCTP code.

Vlad Yasevich

2013-08-10 01:34 UTC #8 lore raw

Re: [PATCH] net: sctp: fix panic during skb_orphan()

Quoted 9 lines · Eric Dumazet

Quoted 6 lines · Vlad Yasevich
Quoted 1 line · Eric Dumazet
Why not using a dummy destructor ?
It would just be useless once we do proper accounting for all chunks. But, I'll send a different idea if this is just too ugly.
Setting skb->sk is no accounting.

Right. It is only really used right now when constructing a packet to transmit out of a set of chunks.

Quoted 3 lines

It seems you would better use skb->cb[] so that you are 100% sure this skb->sk doesn't leak outside of SCTP code.

Hmm.. That might work too. I am building a framework right now that we can use to add proper buffer accounting. That's probably a better long term solution.

-vlad

Vlad Yasevich

2013-08-10 02:05 UTC #9 lore raw

PATCH 1/1 net: sctp: Add rudimentary infrastructure to account for control chunks

In reply to Vlad Yasevich

This patch adds a base infrastructure that allows SCTP to do memory accounting for control chunks. Real accounting code will follow.

This patch alos fixes the following triggered bug ...

[  553.109742] kernel BUG at include/linux/skbuff.h:1813!
[  553.109766] invalid opcode: 0000 [#1] SMP
[  553.109789] Modules linked in: sctp libcrc32c rfcomm [...]
[  553.110259]  uinput i915 i2c_algo_bit drm_kms_helper e1000e drm ptp
pps_core i2c_core wmi video sunrpc
[  553.110320] CPU: 0 PID: 1636 Comm: lt-test_1_to_1_ Not tainted
3.11.0-rc3+ #2
[  553.110350] Hardware name: LENOVO 74597D6/74597D6, BIOS 6DET60WW
(3.10 ) 09/17/2009
[  553.110381] task: ffff88020a01dd40 ti: ffff880204ed0000 task.ti:
ffff880204ed0000
[  553.110411] RIP: 0010:[<ffffffffa0698017>]  [<ffffffffa0698017>]
skb_orphan.part.9+0x4/0x6 [sctp]
[  553.110459] RSP: 0018:ffff880204ed1bb8  EFLAGS: 00010286
[  553.110483] RAX: ffff8802086f5a40 RBX: ffff880204303300 RCX:
0000000000000000
[  553.110487] RDX: ffff880204303c28 RSI: ffff8802086f5a40 RDI:
ffff880202158000
[  553.110487] RBP: ffff880204ed1bb8 R08: 0000000000000000 R09:
0000000000000000
[  553.110487] R10: ffff88022f2d9a04 R11: ffff880233001600 R12:
0000000000000000
[  553.110487] R13: ffff880204303c00 R14: ffff8802293d0000 R15:
ffff880202158000
[  553.110487] FS:  00007f31b31fe740(0000) GS:ffff88023bc00000(0000)
knlGS:0000000000000000
[  553.110487] CS:  0010 DS: 0000 ES: 0000 CR0: 000000008005003b
[  553.110487] CR2: 000000379980e3e0 CR3: 000000020d225000 CR4:
00000000000407f0
[  553.110487] Stack:
[  553.110487]  ffff880204ed1ca8 ffffffffa068d7fc 0000000000000000
0000000000000000
[  553.110487]  0000000000000000 ffff8802293d0000 ffff880202158000
ffffffff81cb7900
[  553.110487]  0000000000000000 0000400000001c68 ffff8802086f5a40
000000000000000f
[  553.110487] Call Trace:
[  553.110487]  [<ffffffffa068d7fc>] sctp_sendmsg+0x6bc/0xc80 [sctp]
[  553.110487]  [<ffffffff8128f185>] ? sock_has_perm+0x75/0x90
[  553.110487]  [<ffffffff815a3593>] inet_sendmsg+0x63/0xb0
[  553.110487]  [<ffffffff8128f2b3>] ? selinux_socket_sendmsg+0x23/0x30
[  553.110487]  [<ffffffff8151c5d6>] sock_sendmsg+0xa6/0xd0
[  553.110487]  [<ffffffff81637b05>] ? _raw_spin_unlock_bh+0x15/0x20
[  553.110487]  [<ffffffff8151cd38>] SYSC_sendto+0x128/0x180
[  553.110487]  [<ffffffff8151ce6b>] ? SYSC_connect+0xdb/0x100
[  553.110487]  [<ffffffffa0690031>] ? sctp_inet_listen+0x71/0x1f0
[sctp]
[  553.110487]  [<ffffffff8151d35e>] SyS_sendto+0xe/0x10
[  553.110487]  [<ffffffff81640202>] system_call_fastpath+0x16/0x1b
[  553.110487] Code: e0 48 c7 c7 00 22 6a a0 e8 67 a3 f0 e0 48 c7 [...]
[  553.110487] RIP  [<ffffffffa0698017>] skb_orphan.part.9+0x4/0x6
[sctp]
[  553.110487]  RSP <ffff880204ed1bb8>
[  553.121578] ---[ end trace 46c20c5903ef5be2 ]---

The approach taken here is to split data and control chunks creation a bit. Data chunks already have memory accounting so noting needs to happen. For control chunks, add stubs handlers.

Signed-off-by Vlad Yasevich <[email protected]>

---
 net/sctp/sm_make_chunk.c | 99 +++++++++++++++++++++++++++++++++---------------
 1 file changed, 69 insertions(+), 30 deletions(-)

net/sctp/sm_make_chunk.c +31 -5

index 780a2d4..04a82a6 100644
@@ -68,8 +68,12 @@
 #include <net/sctp/sctp.h>
 #include <net/sctp/sm.h>
 
-static struct sctp_chunk *sctp_make_chunk(const struct sctp_association *asoc,
-					  __u8 type, __u8 flags, int paylen);
+static struct sctp_chunk *sctp_make_control(const struct sctp_association *asoc,
+					    __u8 type, __u8 flags, int paylen);
+static struct sctp_chunk *sctp_make_data(const struct sctp_association *asoc,
+					 __u8 flags, int paylen);
+static struct sctp_chunk *_sctp_make_chunk(const struct sctp_association *asoc,
+					   __u8 type, __u8 flags, int paylen);
 static sctp_cookie_param_t *sctp_pack_cookie(const struct sctp_endpoint *ep,
 					const struct sctp_association *asoc,
 					const struct sctp_chunk *init_chunk,

@@ -82,6 +86,28 @@ static int sctp_process_param(struct sctp_association *asoc,
 static void *sctp_addto_param(struct sctp_chunk *chunk, int len,
 			      const void *data);
 
+/* Control chunk destructor */
+static void sctp_control_release_owner(struct sk_buff *skb)
+{
+	/*TODO: do memory release */
+}
+
+static void sctp_control_set_owner_w(struct sctp_chunk *chunk)
+{
+	struct sctp_association *asoc = chunk->asoc;
+	struct sk_buff *skb = chunk->skb;
+
+	/* TODO: properly account for control chunks. 
+	 * To do it right we'll need:
+	 *  1) endpoint if association isn't known.
+	 *  2) proper memory accounting.
+	 *
+	 *  For now don't do anything for now.
+	 */
+	skb->sk = asoc ? asoc->base.sk : NULL;
+	skb->destructor = sctp_control_release_owner;
+}
+
 /* What was the inbound interface for this chunk? */
 int sctp_chunk_iif(const struct sctp_chunk *chunk)
 {

@@ -296,7 +322,7 @@ struct sctp_chunk *sctp_make_init(const struct sctp_association *asoc,
 	 * PLEASE DO NOT FIXME [This version does not support Host Name.]
 	 */
 
-	retval = sctp_make_chunk(asoc, SCTP_CID_INIT, 0, chunksize);
+	retval = sctp_make_control(asoc, SCTP_CID_INIT, 0, chunksize);
 	if (!retval)
 		goto nodata;

@@ -443,7 +469,7 @@ struct sctp_chunk *sctp_make_init_ack(const struct sctp_association *asoc,
 					num_ext);
 
 	/* Now allocate and fill out the chunk.  */
-	retval = sctp_make_chunk(asoc, SCTP_CID_INIT_ACK, 0, chunksize);
+	retval = sctp_make_control(asoc, SCTP_CID_INIT_ACK, 0, chunksize);
 	if (!retval)
 		goto nomem_chunk;

@@ -548,7 +574,7 @@ struct sctp_chunk *sctp_make_cookie_echo(const struct sctp_association *asoc,
 	cookie_len = asoc->peer.cookie_len;
 
 	/* Build a cookie echo chunk.  */
-	retval = sctp_make_chunk(asoc, SCTP_CID_COOKIE_ECHO, 0, cookie_len);
+	retval = sctp_make_control(asoc, SCTP_CID_COOKIE_ECHO, 0, cookie_len);
 	if (!retval)
 		goto nodata;
 	retval->subh.cookie_hdr @@ -593,7 +619,7 @@ struct sctp_chunk *sctp_make_cookie_ack(const struct sctp_association *asoc,

{
struct sctp_chunk *retval;

- retval = sctp_make_chunk(asoc, SCTP_CID_COOKIE_ACK, 0, 0);
+ retval = sctp_make_control(asoc, SCTP_CID_COOKIE_ACK, 0, 0);

/* RFC 2960 6.4 Multi-homed SCTP Endpoints
*
@@ -641,8 +667,8 @@ struct sctp_chunk *sctp_make_cwr(const struct sctp_association *asoc,
sctp_cwrhdr_t cwr;

cwr.lowest_tsn = htonl(lowest_tsn);
- retval = sctp_make_chunk(asoc, SCTP_CID_ECN_CWR, 0,
- sizeof(sctp_cwrhdr_t));
+ retval = sctp_make_control(asoc, SCTP_CID_ECN_CWR, 0,
+ sizeof(sctp_cwrhdr_t));

if (!retval)
goto nodata;
@@ -675,8 +701,8 @@ struct sctp_chunk *sctp_make_ecne(const struct sctp_association *asoc,
sctp_ecnehdr_t ecne;

ecne.lowest_tsn = htonl(lowest_tsn);
- retval = sctp_make_chunk(asoc, SCTP_CID_ECN_ECNE, 0,
- sizeof(sctp_ecnehdr_t));
+ retval = sctp_make_control(asoc, SCTP_CID_ECN_ECNE, 0,
+ sizeof(sctp_ecnehdr_t));
if (!retval)
goto nodata;
retval->subh.ecne_hdr @@ -712,7 +738,7 @@ struct sctp_chunk *sctp_make_datafrag_empty(struct sctp_association *asoc,
dp.ssn = htons(ssn);

chunk_len = sizeof(dp) + data_len;
- retval = sctp_make_chunk(asoc, SCTP_CID_DATA, flags, chunk_len);
+ retval = sctp_make_data(asoc, flags, chunk_len);
if (!retval)
goto nodata;

@@ -759,7 +785,7 @@ struct sctp_chunk *sctp_make_sack(const struct sctp_association *asoc)
+ sizeof(__u32) * num_dup_tsns;

/* Create the chunk. */
- retval = sctp_make_chunk(asoc, SCTP_CID_SACK, 0, len);
+ retval = sctp_make_control(asoc, SCTP_CID_SACK, 0, len);
if (!retval)
goto nodata;

@@ -838,8 +864,8 @@ struct sctp_chunk *sctp_make_shutdown(const struct sctp_association *asoc,
ctsn = sctp_tsnmap_get_ctsn(&asoc->peer.tsn_map);
shut.cum_tsn_ack = htonl(ctsn);

- retval = sctp_make_chunk(asoc, SCTP_CID_SHUTDOWN, 0,
- sizeof(sctp_shutdownhdr_t));
+ retval = sctp_make_control(asoc, SCTP_CID_SHUTDOWN, 0,
+ sizeof(sctp_shutdownhdr_t));
if (!retval)
goto nodata;

@@ -857,7 +883,7 @@ struct sctp_chunk *sctp_make_shutdown_ack(const struct sctp_association *asoc,
{
struct sctp_chunk *retval;

- retval = sctp_make_chunk(asoc, SCTP_CID_SHUTDOWN_ACK, 0, 0);
+ retval = sctp_make_control(asoc, SCTP_CID_SHUTDOWN_ACK, 0, 0);

/* RFC 2960 6.4 Multi-homed SCTP Endpoints
*
@@ -886,7 +912,7 @@ struct sctp_chunk *sctp_make_shutdown_complete(
*/
flags |= asoc ? 0 : SCTP_CHUNK_FLAG_T;

- retval = sctp_make_chunk(asoc, SCTP_CID_SHUTDOWN_COMPLETE, flags, 0);
+ retval = sctp_make_control(asoc, SCTP_CID_SHUTDOWN_COMPLETE, flags, 0);

/* RFC 2960 6.4 Multi-homed SCTP Endpoints
*
@@ -925,7 +951,7 @@ struct sctp_chunk *sctp_make_abort(const struct sctp_association *asoc,
flags = SCTP_CHUNK_FLAG_T;
}

- retval = sctp_make_chunk(asoc, SCTP_CID_ABORT, flags, hint);
+ retval = sctp_make_control(asoc, SCTP_CID_ABORT, flags, hint);

/* RFC 2960 6.4 Multi-homed SCTP Endpoints
*
@@ -1117,7 +1143,7 @@ struct sctp_chunk *sctp_make_heartbeat(const struct sctp_association *asoc,
struct sctp_chunk *retval;
sctp_sender_hb_info_t hbinfo;

- retval = sctp_make_chunk(asoc, SCTP_CID_HEARTBEAT, 0, sizeof(hbinfo));
+ retval = sctp_make_control(asoc, SCTP_CID_HEARTBEAT, 0, sizeof(hbinfo));

if (!retval)
goto nodata;
@@ -1145,7 +1171,7 @@ struct sctp_chunk *sctp_make_heartbeat_ack(const struct sctp_association *asoc,
{
struct sctp_chunk *retval;

- retval = sctp_make_chunk(asoc, SCTP_CID_HEARTBEAT_ACK, 0, paylen);
+ retval = sctp_make_control(asoc, SCTP_CID_HEARTBEAT_ACK, 0, paylen);
if (!retval)
goto nodata;

@@ -1177,8 +1203,8 @@ static struct sctp_chunk *sctp_make_op_error_space(
{
struct sctp_chunk *retval;

- retval = sctp_make_chunk(asoc, SCTP_CID_ERROR, 0,
- sizeof(sctp_errhdr_t) + size);
+ retval = sctp_make_control(asoc, SCTP_CID_ERROR, 0,
+ sizeof(sctp_errhdr_t) + size);
if (!retval)
goto nodata;

@@ -1248,7 +1274,7 @@ struct sctp_chunk *sctp_make_auth(const struct sctp_association *asoc)
if (unlikely(!hmac_desc))
return NULL;

- retval = sctp_make_chunk(asoc, SCTP_CID_AUTH, 0,
+ retval = sctp_make_control(asoc, SCTP_CID_AUTH, 0,
hmac_desc->hmac_len + sizeof(sctp_authhdr_t));
if (!retval)
return NULL;
@@ -1351,8 +1377,8 @@ const union sctp_addr *sctp_source(const struct sctp_chunk *chunk)
/* Create a new chunk, setting the type and flags headers from the
* arguments, reserving enough space for a 'paylen' byte payload.
*/
-static struct sctp_chunk *sctp_make_chunk(const struct sctp_association *asoc,
- __u8 type, __u8 flags, int paylen)
+static struct sctp_chunk *_sctp_make_chunk(const struct sctp_association *asoc,
+ __u8 type, __u8 flags, int paylen)
{
struct sctp_chunk *retval;
sctp_chunkhdr_t *chunk_hdr;
@@ -1385,14 +1411,27 @@ static struct sctp_chunk *sctp_make_chunk(const struct sctp_association *asoc,
if (sctp_auth_send_cid(type, asoc))
retval->auth = 1;

- /* Set the skb to the belonging sock for accounting. */
- skb->sk = sk;
-
return retval;
nodata:
return NULL;
}

+static struct sctp_chunk *sctp_make_data(const struct sctp_association *asoc,
+ __u8 flags, int paylen)
+{
+ return _sctp_make_chunk(asoc, SCTP_CID_DATA, flags, paylen);
+}
+
+static struct sctp_chunk *sctp_make_control(const struct sctp_association *asoc,
+ __u8 type, __u8 flags, int paylen)
+{
+ struct sctp_chunk *chunk = _sctp_make_chunk(asoc, type, flags, paylen);
+
+ if (chunk)
+ sctp_control_set_owner_w(chunk);
+
+ return chunk;
+}

/* Release the memory occupied by a chunk. */
static void sctp_chunk_destroy(struct sctp_chunk *chunk)
@@ -2733,7 +2772,7 @@ static struct sctp_chunk *sctp_make_asconf(struct sctp_association *asoc,
length += addrlen;

/* Create the chunk. */
- retval = sctp_make_chunk(asoc, SCTP_CID_ASCONF, 0, length);
+ retval = sctp_make_control(asoc, SCTP_CID_ASCONF, 0, length);
if (!retval)
return NULL;

@@ -2917,7 +2956,7 @@ static struct sctp_chunk *sctp_make_asconf_ack(const struct sctp_association *as
int length = sizeof(asconf) + vparam_len;

/* Create the chunk. */
- retval = sctp_make_chunk(asoc, SCTP_CID_ASCONF_ACK, 0, length);
+ retval = sctp_make_control(asoc, SCTP_CID_ASCONF_ACK, 0, length);
if (!retval)
return NULL;

@@ -3448,7 +3487,7 @@ struct sctp_chunk *sctp_make_fwdtsn(const struct sctp_association *asoc,

hint = (nstreams + 1) * sizeof(__u32);

- retval = sctp_make_chunk(asoc, SCTP_CID_FWD_TSN, 0, hint);
+ retval = sctp_make_control(asoc, SCTP_CID_FWD_TSN, 0, hint);

if (!retval)
return NULL;

Signature · 1 line

1.8.1.4

David Miller

maintainer Applied

2013-08-13 22:04 UTC #10 lore raw

Re: [PATCH net-next] net: sctp: Add rudimentary infrastructure to account for control chunks

From: Vlad Yasevich <[email protected]>
Date: Fri, 9 Aug 2013 22:05:36 -0400

Quoted 5 lines

This patch adds a base infrastructure that allows SCTP to do memory accounting for control chunks. Real accounting code will follow.
This patch alos fixes the following triggered bug ...

...

Quoted 5 lines

The approach taken here is to split data and control chunks creation a bit. Data chunks already have memory accounting so noting needs to happen. For control chunks, add stubs handlers.
Signed-off-by Vlad Yasevich <[email protected]>

Applied, but:

net/sctp/sm_make_chunk.c static int sctp_process_param(struct sctp_association *asoc, · line 100

Quoted 1 line

net/sctp/sm_make_chunk.c +1 -0
+	/* TODO: properly account for control chunks. 

I had to delete the trailing whitespace on that line.