]>
Commit | Line | Data |
---|---|---|
8af540ce GKH |
1 | From foo@baz Sun 09 Jun 2019 09:24:16 AM CEST |
2 | From: Neil Horman <nhorman@tuxdriver.com> | |
3 | Date: Mon, 3 Jun 2019 16:32:59 -0400 | |
4 | Subject: Fix memory leak in sctp_process_init | |
5 | ||
6 | From: Neil Horman <nhorman@tuxdriver.com> | |
7 | ||
8 | [ Upstream commit 0a8dd9f67cd0da7dc284f48b032ce00db1a68791 ] | |
9 | ||
10 | syzbot found the following leak in sctp_process_init | |
11 | BUG: memory leak | |
12 | unreferenced object 0xffff88810ef68400 (size 1024): | |
13 | comm "syz-executor273", pid 7046, jiffies 4294945598 (age 28.770s) | |
14 | hex dump (first 32 bytes): | |
15 | 1d de 28 8d de 0b 1b e3 b5 c2 f9 68 fd 1a 97 25 ..(........h...% | |
16 | 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................ | |
17 | backtrace: | |
18 | [<00000000a02cebbd>] kmemleak_alloc_recursive include/linux/kmemleak.h:55 | |
19 | [inline] | |
20 | [<00000000a02cebbd>] slab_post_alloc_hook mm/slab.h:439 [inline] | |
21 | [<00000000a02cebbd>] slab_alloc mm/slab.c:3326 [inline] | |
22 | [<00000000a02cebbd>] __do_kmalloc mm/slab.c:3658 [inline] | |
23 | [<00000000a02cebbd>] __kmalloc_track_caller+0x15d/0x2c0 mm/slab.c:3675 | |
24 | [<000000009e6245e6>] kmemdup+0x27/0x60 mm/util.c:119 | |
25 | [<00000000dfdc5d2d>] kmemdup include/linux/string.h:432 [inline] | |
26 | [<00000000dfdc5d2d>] sctp_process_init+0xa7e/0xc20 | |
27 | net/sctp/sm_make_chunk.c:2437 | |
28 | [<00000000b58b62f8>] sctp_cmd_process_init net/sctp/sm_sideeffect.c:682 | |
29 | [inline] | |
30 | [<00000000b58b62f8>] sctp_cmd_interpreter net/sctp/sm_sideeffect.c:1384 | |
31 | [inline] | |
32 | [<00000000b58b62f8>] sctp_side_effects net/sctp/sm_sideeffect.c:1194 | |
33 | [inline] | |
34 | [<00000000b58b62f8>] sctp_do_sm+0xbdc/0x1d60 net/sctp/sm_sideeffect.c:1165 | |
35 | [<0000000044e11f96>] sctp_assoc_bh_rcv+0x13c/0x200 | |
36 | net/sctp/associola.c:1074 | |
37 | [<00000000ec43804d>] sctp_inq_push+0x7f/0xb0 net/sctp/inqueue.c:95 | |
38 | [<00000000726aa954>] sctp_backlog_rcv+0x5e/0x2a0 net/sctp/input.c:354 | |
39 | [<00000000d9e249a8>] sk_backlog_rcv include/net/sock.h:950 [inline] | |
40 | [<00000000d9e249a8>] __release_sock+0xab/0x110 net/core/sock.c:2418 | |
41 | [<00000000acae44fa>] release_sock+0x37/0xd0 net/core/sock.c:2934 | |
42 | [<00000000963cc9ae>] sctp_sendmsg+0x2c0/0x990 net/sctp/socket.c:2122 | |
43 | [<00000000a7fc7565>] inet_sendmsg+0x64/0x120 net/ipv4/af_inet.c:802 | |
44 | [<00000000b732cbd3>] sock_sendmsg_nosec net/socket.c:652 [inline] | |
45 | [<00000000b732cbd3>] sock_sendmsg+0x54/0x70 net/socket.c:671 | |
46 | [<00000000274c57ab>] ___sys_sendmsg+0x393/0x3c0 net/socket.c:2292 | |
47 | [<000000008252aedb>] __sys_sendmsg+0x80/0xf0 net/socket.c:2330 | |
48 | [<00000000f7bf23d1>] __do_sys_sendmsg net/socket.c:2339 [inline] | |
49 | [<00000000f7bf23d1>] __se_sys_sendmsg net/socket.c:2337 [inline] | |
50 | [<00000000f7bf23d1>] __x64_sys_sendmsg+0x23/0x30 net/socket.c:2337 | |
51 | [<00000000a8b4131f>] do_syscall_64+0x76/0x1a0 arch/x86/entry/common.c:3 | |
52 | ||
53 | The problem was that the peer.cookie value points to an skb allocated | |
54 | area on the first pass through this function, at which point it is | |
55 | overwritten with a heap allocated value, but in certain cases, where a | |
56 | COOKIE_ECHO chunk is included in the packet, a second pass through | |
57 | sctp_process_init is made, where the cookie value is re-allocated, | |
58 | leaking the first allocation. | |
59 | ||
60 | Fix is to always allocate the cookie value, and free it when we are done | |
61 | using it. | |
62 | ||
63 | Signed-off-by: Neil Horman <nhorman@tuxdriver.com> | |
64 | Reported-by: syzbot+f7e9153b037eac9b1df8@syzkaller.appspotmail.com | |
65 | CC: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> | |
66 | CC: "David S. Miller" <davem@davemloft.net> | |
67 | CC: netdev@vger.kernel.org | |
68 | Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com> | |
69 | Signed-off-by: David S. Miller <davem@davemloft.net> | |
70 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | |
71 | --- | |
72 | net/sctp/sm_make_chunk.c | 13 +++---------- | |
73 | net/sctp/sm_sideeffect.c | 5 +++++ | |
74 | 2 files changed, 8 insertions(+), 10 deletions(-) | |
75 | ||
76 | --- a/net/sctp/sm_make_chunk.c | |
77 | +++ b/net/sctp/sm_make_chunk.c | |
78 | @@ -2329,7 +2329,6 @@ int sctp_process_init(struct sctp_associ | |
79 | union sctp_addr addr; | |
80 | struct sctp_af *af; | |
81 | int src_match = 0; | |
82 | - char *cookie; | |
83 | ||
84 | /* We must include the address that the INIT packet came from. | |
85 | * This is the only address that matters for an INIT packet. | |
86 | @@ -2433,14 +2432,6 @@ int sctp_process_init(struct sctp_associ | |
87 | /* Peer Rwnd : Current calculated value of the peer's rwnd. */ | |
88 | asoc->peer.rwnd = asoc->peer.i.a_rwnd; | |
89 | ||
90 | - /* Copy cookie in case we need to resend COOKIE-ECHO. */ | |
91 | - cookie = asoc->peer.cookie; | |
92 | - if (cookie) { | |
93 | - asoc->peer.cookie = kmemdup(cookie, asoc->peer.cookie_len, gfp); | |
94 | - if (!asoc->peer.cookie) | |
95 | - goto clean_up; | |
96 | - } | |
97 | - | |
98 | /* RFC 2960 7.2.1 The initial value of ssthresh MAY be arbitrarily | |
99 | * high (for example, implementations MAY use the size of the receiver | |
100 | * advertised window). | |
101 | @@ -2609,7 +2600,9 @@ do_addr_param: | |
102 | case SCTP_PARAM_STATE_COOKIE: | |
103 | asoc->peer.cookie_len = | |
104 | ntohs(param.p->length) - sizeof(struct sctp_paramhdr); | |
105 | - asoc->peer.cookie = param.cookie->body; | |
106 | + asoc->peer.cookie = kmemdup(param.cookie->body, asoc->peer.cookie_len, gfp); | |
107 | + if (!asoc->peer.cookie) | |
108 | + retval = 0; | |
109 | break; | |
110 | ||
111 | case SCTP_PARAM_HEARTBEAT_INFO: | |
112 | --- a/net/sctp/sm_sideeffect.c | |
113 | +++ b/net/sctp/sm_sideeffect.c | |
114 | @@ -898,6 +898,11 @@ static void sctp_cmd_new_state(struct sc | |
115 | asoc->rto_initial; | |
116 | } | |
117 | ||
118 | + if (sctp_state(asoc, ESTABLISHED)) { | |
119 | + kfree(asoc->peer.cookie); | |
120 | + asoc->peer.cookie = NULL; | |
121 | + } | |
122 | + | |
123 | if (sctp_state(asoc, ESTABLISHED) || | |
124 | sctp_state(asoc, CLOSED) || | |
125 | sctp_state(asoc, SHUTDOWN_RECEIVED)) { |