Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

sctp: add a refcnt in sctp_stream_priorities to avoid a nested loop

With this refcnt added in sctp_stream_priorities, we don't need to
traverse all streams to check if the prio is used by other streams
when freeing one stream's prio in sctp_sched_prio_free_sid(). This
can avoid a nested loop (up to 65535 * 65535), which may cause a
stuck as Ying reported:

watchdog: BUG: soft lockup - CPU#23 stuck for 26s! [ksoftirqd/23:136]
Call Trace:
<TASK>
sctp_sched_prio_free_sid+0xab/0x100 [sctp]
sctp_stream_free_ext+0x64/0xa0 [sctp]
sctp_stream_free+0x31/0x50 [sctp]
sctp_association_free+0xa5/0x200 [sctp]

Note that it doesn't need to use refcount_t type for this counter,
as its accessing is always protected under the sock lock.

v1->v2:
- add a check in sctp_sched_prio_set to avoid the possible prio_head
refcnt overflow.

Fixes: 9ed7bfc79542 ("sctp: fix memory leak in sctp_stream_outq_migrate()")
Reported-by: Ying Xu <yinxu@redhat.com>
Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
Link: https://lore.kernel.org/r/825eb0c905cb864991eba335f4a2b780e543f06b.1677085641.git.lucien.xin@gmail.com
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

authored by

Xin Long and committed by
Jakub Kicinski
68ba4463 ee0a735f

+22 -31
+1
include/net/sctp/structs.h
··· 1412 1412 /* The next stream in line */ 1413 1413 struct sctp_stream_out_ext *next; 1414 1414 __u16 prio; 1415 + __u16 users; 1415 1416 }; 1416 1417 1417 1418 struct sctp_stream_out_ext {
+21 -31
net/sctp/stream_sched_prio.c
··· 25 25 26 26 static void sctp_sched_prio_unsched_all(struct sctp_stream *stream); 27 27 28 + static struct sctp_stream_priorities *sctp_sched_prio_head_get(struct sctp_stream_priorities *p) 29 + { 30 + p->users++; 31 + return p; 32 + } 33 + 34 + static void sctp_sched_prio_head_put(struct sctp_stream_priorities *p) 35 + { 36 + if (p && --p->users == 0) 37 + kfree(p); 38 + } 39 + 28 40 static struct sctp_stream_priorities *sctp_sched_prio_new_head( 29 41 struct sctp_stream *stream, int prio, gfp_t gfp) 30 42 { ··· 50 38 INIT_LIST_HEAD(&p->active); 51 39 p->next = NULL; 52 40 p->prio = prio; 41 + p->users = 1; 53 42 54 43 return p; 55 44 } ··· 66 53 */ 67 54 list_for_each_entry(p, &stream->prio_list, prio_sched) { 68 55 if (p->prio == prio) 69 - return p; 56 + return sctp_sched_prio_head_get(p); 70 57 if (p->prio > prio) 71 58 break; 72 59 } ··· 83 70 */ 84 71 break; 85 72 if (p->prio == prio) 86 - return p; 73 + return sctp_sched_prio_head_get(p); 87 74 } 88 75 89 76 /* If not even there, allocate a new one. */ ··· 167 154 struct sctp_stream_out_ext *soute = sout->ext; 168 155 struct sctp_stream_priorities *prio_head, *old; 169 156 bool reschedule = false; 170 - int i; 157 + 158 + old = soute->prio_head; 159 + if (old && old->prio == prio) 160 + return 0; 171 161 172 162 prio_head = sctp_sched_prio_get_head(stream, prio, gfp); 173 163 if (!prio_head) 174 164 return -ENOMEM; 175 165 176 166 reschedule = sctp_sched_prio_unsched(soute); 177 - old = soute->prio_head; 178 167 soute->prio_head = prio_head; 179 168 if (reschedule) 180 169 sctp_sched_prio_sched(stream, soute); 181 170 182 - if (!old) 183 - /* Happens when we set the priority for the first time */ 184 - return 0; 185 - 186 - for (i = 0; i < stream->outcnt; i++) { 187 - soute = SCTP_SO(stream, i)->ext; 188 - if (soute && soute->prio_head == old) 189 - /* It's still in use, nothing else to do here. */ 190 - return 0; 191 - } 192 - 193 - /* No hits, we are good to free it. */ 194 - kfree(old); 195 - 171 + sctp_sched_prio_head_put(old); 196 172 return 0; 197 173 } 198 174 ··· 208 206 209 207 static void sctp_sched_prio_free_sid(struct sctp_stream *stream, __u16 sid) 210 208 { 211 - struct sctp_stream_priorities *prio = SCTP_SO(stream, sid)->ext->prio_head; 212 - int i; 213 - 214 - if (!prio) 215 - return; 216 - 209 + sctp_sched_prio_head_put(SCTP_SO(stream, sid)->ext->prio_head); 217 210 SCTP_SO(stream, sid)->ext->prio_head = NULL; 218 - for (i = 0; i < stream->outcnt; i++) { 219 - if (SCTP_SO(stream, i)->ext && 220 - SCTP_SO(stream, i)->ext->prio_head == prio) 221 - return; 222 - } 223 - 224 - kfree(prio); 225 211 } 226 212 227 213 static void sctp_sched_prio_enqueue(struct sctp_outq *q,