From: Xin Long lucien.xin@gmail.com
[ Upstream commit 68ba44639537de6f91fe32783766322d41848127 ]
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.167708564... Signed-off-by: Jakub Kicinski kuba@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org --- include/net/sctp/structs.h | 1 + net/sctp/stream_sched_prio.c | 52 +++++++++++++++--------------------- 2 files changed, 22 insertions(+), 31 deletions(-)
diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h index cb05e503c9cd1..48cbf3352042f 100644 --- a/include/net/sctp/structs.h +++ b/include/net/sctp/structs.h @@ -1400,6 +1400,7 @@ struct sctp_stream_priorities { /* The next stream stream in line */ struct sctp_stream_out_ext *next; __u16 prio; + __u16 users; };
struct sctp_stream_out_ext { diff --git a/net/sctp/stream_sched_prio.c b/net/sctp/stream_sched_prio.c index 4fc9f2923ed11..7dd9f8b387cca 100644 --- a/net/sctp/stream_sched_prio.c +++ b/net/sctp/stream_sched_prio.c @@ -25,6 +25,18 @@
static void sctp_sched_prio_unsched_all(struct sctp_stream *stream);
+static struct sctp_stream_priorities *sctp_sched_prio_head_get(struct sctp_stream_priorities *p) +{ + p->users++; + return p; +} + +static void sctp_sched_prio_head_put(struct sctp_stream_priorities *p) +{ + if (p && --p->users == 0) + kfree(p); +} + static struct sctp_stream_priorities *sctp_sched_prio_new_head( struct sctp_stream *stream, int prio, gfp_t gfp) { @@ -38,6 +50,7 @@ static struct sctp_stream_priorities *sctp_sched_prio_new_head( INIT_LIST_HEAD(&p->active); p->next = NULL; p->prio = prio; + p->users = 1;
return p; } @@ -53,7 +66,7 @@ static struct sctp_stream_priorities *sctp_sched_prio_get_head( */ list_for_each_entry(p, &stream->prio_list, prio_sched) { if (p->prio == prio) - return p; + return sctp_sched_prio_head_get(p); if (p->prio > prio) break; } @@ -70,7 +83,7 @@ static struct sctp_stream_priorities *sctp_sched_prio_get_head( */ break; if (p->prio == prio) - return p; + return sctp_sched_prio_head_get(p); }
/* If not even there, allocate a new one. */ @@ -154,32 +167,21 @@ static int sctp_sched_prio_set(struct sctp_stream *stream, __u16 sid, struct sctp_stream_out_ext *soute = sout->ext; struct sctp_stream_priorities *prio_head, *old; bool reschedule = false; - int i; + + old = soute->prio_head; + if (old && old->prio == prio) + return 0;
prio_head = sctp_sched_prio_get_head(stream, prio, gfp); if (!prio_head) return -ENOMEM;
reschedule = sctp_sched_prio_unsched(soute); - old = soute->prio_head; soute->prio_head = prio_head; if (reschedule) sctp_sched_prio_sched(stream, soute);
- if (!old) - /* Happens when we set the priority for the first time */ - return 0; - - for (i = 0; i < stream->outcnt; i++) { - soute = SCTP_SO(stream, i)->ext; - if (soute && soute->prio_head == old) - /* It's still in use, nothing else to do here. */ - return 0; - } - - /* No hits, we are good to free it. */ - kfree(old); - + sctp_sched_prio_head_put(old); return 0; }
@@ -206,20 +208,8 @@ static int sctp_sched_prio_init_sid(struct sctp_stream *stream, __u16 sid,
static void sctp_sched_prio_free_sid(struct sctp_stream *stream, __u16 sid) { - struct sctp_stream_priorities *prio = SCTP_SO(stream, sid)->ext->prio_head; - int i; - - if (!prio) - return; - + sctp_sched_prio_head_put(SCTP_SO(stream, sid)->ext->prio_head); SCTP_SO(stream, sid)->ext->prio_head = NULL; - for (i = 0; i < stream->outcnt; i++) { - if (SCTP_SO(stream, i)->ext && - SCTP_SO(stream, i)->ext->prio_head == prio) - return; - } - - kfree(prio); }
static void sctp_sched_prio_free(struct sctp_stream *stream)