netdev - [PATCH] sctp: relese sctp_stream_priorities at sctp_stream_outq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <c5ba2194-dbb6-586d-992d-9dfcd27062e7@I-love.SAKURA.ne.jp>
Date:   Wed, 23 Nov 2022 19:36:00 +0900
From:   Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
To:     syzbot <syzbot+29c402e56c4760763cc0@...kaller.appspotmail.com>,
        syzkaller-bugs@...glegroups.com,
        Vlad Yasevich <vyasevich@...il.com>,
        Neil Horman <nhorman@...driver.com>,
        Marcelo Ricardo Leitner <marcelo.leitner@...il.com>,
        linux-sctp@...r.kernel.org
Cc:     davem@...emloft.net, kuba@...nel.org, netdev@...r.kernel.org,
        pabeni@...hat.com
Subject: [PATCH] sctp: relese sctp_stream_priorities at
 sctp_stream_outq_migrate()

syzbot is reporting memory leak on sctp_stream_priorities [1], for
sctp_stream_outq_migrate() is resetting SCTP_SO(stream, i)->ext to NULL
without clearing SCTP_SO(new, i)->ext->prio_head list allocated by
sctp_sched_prio_new_head(). Since sctp_sched_prio_free() is too late to
clear if stream->outcnt was already shrunk or SCTP_SO(stream, i)->ext
was already NULL, add a callback for clearing that list before shrinking
stream->outcnt and/or resetting SCTP_SO(stream, i)->ext.

Link: https://syzkaller.appspot.com/bug?exrid=29c402e56c4760763cc0 [1]
Reported-by: syzbot <syzbot+29c402e56c4760763cc0@...kaller.appspotmail.com>
Signed-off-by: Tetsuo Handa <penguin-kernel@...ove.SAKURA.ne.jp>
---
I can observe that the reproducer no longer reports memory leak. But
is this change correct and sufficient? Are there similar locations?

 include/net/sctp/stream_sched.h |  2 ++
 net/sctp/stream.c               |  3 +++
 net/sctp/stream_sched_prio.c    | 20 ++++++++++++++++++++
 3 files changed, 25 insertions(+)

diff --git a/include/net/sctp/stream_sched.h b/include/net/sctp/stream_sched.h
index 01a70b27e026..1a59d0f8ad79 100644
--- a/include/net/sctp/stream_sched.h
+++ b/include/net/sctp/stream_sched.h
@@ -28,6 +28,8 @@ struct sctp_sched_ops {
 	int (*init_sid)(struct sctp_stream *stream, __u16 sid, gfp_t gfp);
 	/* Frees the entire thing */
 	void (*free)(struct sctp_stream *stream);
+	/* Free one sid */
+	void (*free_sid)(struct sctp_stream *stream, __u16 sid);
 
 	/* Enqueue a chunk */
 	void (*enqueue)(struct sctp_outq *q, struct sctp_datamsg *msg);
diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index ef9fceadef8d..845a8173181e 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -60,6 +60,7 @@ static void sctp_stream_outq_migrate(struct sctp_stream *stream,
 				     struct sctp_stream *new, __u16 outcnt)
 {
 	int i;
+	struct sctp_sched_ops *sched = sctp_sched_ops_from_stream(stream);
 
 	if (stream->outcnt > outcnt)
 		sctp_stream_shrink_out(stream, outcnt);
@@ -77,6 +78,8 @@ static void sctp_stream_outq_migrate(struct sctp_stream *stream,
 	}
 
 	for (i = outcnt; i < stream->outcnt; i++) {
+		if (sched->free_sid)
+			sched->free_sid(stream, i);
 		kfree(SCTP_SO(stream, i)->ext);
 		SCTP_SO(stream, i)->ext = NULL;
 	}
diff --git a/net/sctp/stream_sched_prio.c b/net/sctp/stream_sched_prio.c
index 80b5a2c4cbc7..bde5537984a9 100644
--- a/net/sctp/stream_sched_prio.c
+++ b/net/sctp/stream_sched_prio.c
@@ -230,6 +230,25 @@ static void sctp_sched_prio_free(struct sctp_stream *stream)
 	}
 }
 
+static void sctp_sched_prio_free_sid(struct sctp_stream *stream, __u16 sid)
+{
+	struct sctp_stream_priorities *prio, *n;
+	struct sctp_stream_out *sout = SCTP_SO(stream, sid);
+	struct sctp_stream_out_ext *soute = sout->ext;
+	LIST_HEAD(list);
+
+	if (!soute)
+		return;
+	prio = soute->prio_head;
+	if (!prio || !list_empty(&prio->prio_sched))
+		return;
+	list_add(&prio->prio_sched, &list);
+	list_for_each_entry_safe(prio, n, &list, prio_sched) {
+		list_del_init(&prio->prio_sched);
+		kfree(prio);
+	}
+}
+
 static void sctp_sched_prio_enqueue(struct sctp_outq *q,
 				    struct sctp_datamsg *msg)
 {
@@ -323,6 +342,7 @@ static struct sctp_sched_ops sctp_sched_prio = {
 	.get = sctp_sched_prio_get,
 	.init = sctp_sched_prio_init,
 	.init_sid = sctp_sched_prio_init_sid,
+	.free_sid = sctp_sched_prio_free_sid,
 	.free = sctp_sched_prio_free,
 	.enqueue = sctp_sched_prio_enqueue,
 	.dequeue = sctp_sched_prio_dequeue,
-- 
2.34.1