* [PATCH net] sctp: fix error path in sctp_stream_init
@ 2018-01-02 21:44 Marcelo Ricardo Leitner
2018-01-03 6:45 ` Xin Long
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Marcelo Ricardo Leitner @ 2018-01-02 21:44 UTC (permalink / raw)
To: netdev; +Cc: linux-sctp, Xin Long, Vlad Yasevich, Neil Horman
syzbot noticed a NULL pointer dereference panic in sctp_stream_free()
which was caused by an incomplete error handling in sctp_stream_init().
By not clearing stream->outcnt, it made a for() in sctp_stream_free()
think that it had elements to free, but not, leading to the panic.
As suggested by Xin Long, this patch also simplifies the error path by
moving it to the only if() that uses it.
See-also: https://www.spinics.net/lists/netdev/msg473756.html
See-also: https://www.spinics.net/lists/netdev/msg465024.html
Reported-by: syzbot <syzkaller@googlegroups.com>
Fixes: f952be79cebd ("sctp: introduce struct sctp_stream_out_ext")
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
net/sctp/stream.c | 22 ++++++++++------------
1 file changed, 10 insertions(+), 12 deletions(-)
diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index 76ea66be0bbee7d3f018676d52c8b95ba06dbcb1..524dfeb94c41ab1ac735746a8acf93af1c96ae48 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -156,9 +156,9 @@ int sctp_stream_init(struct sctp_stream *stream, __u16 outcnt, __u16 incnt,
sctp_stream_outq_migrate(stream, NULL, outcnt);
sched->sched_all(stream);
- i = sctp_stream_alloc_out(stream, outcnt, gfp);
- if (i)
- return i;
+ ret = sctp_stream_alloc_out(stream, outcnt, gfp);
+ if (ret)
+ goto out;
stream->outcnt = outcnt;
for (i = 0; i < stream->outcnt; i++)
@@ -170,19 +170,17 @@ int sctp_stream_init(struct sctp_stream *stream, __u16 outcnt, __u16 incnt,
if (!incnt)
goto out;
- i = sctp_stream_alloc_in(stream, incnt, gfp);
- if (i) {
- ret = -ENOMEM;
- goto free;
+ ret = sctp_stream_alloc_in(stream, incnt, gfp);
+ if (ret) {
+ sched->free(stream);
+ kfree(stream->out);
+ stream->out = NULL;
+ stream->outcnt = 0;
+ goto out;
}
stream->incnt = incnt;
- goto out;
-free:
- sched->free(stream);
- kfree(stream->out);
- stream->out = NULL;
out:
return ret;
}
--
2.14.3
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH net] sctp: fix error path in sctp_stream_init
2018-01-02 21:44 [PATCH net] sctp: fix error path in sctp_stream_init Marcelo Ricardo Leitner
@ 2018-01-03 6:45 ` Xin Long
2018-01-03 10:32 ` Neil Horman
2018-01-03 16:30 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Xin Long @ 2018-01-03 6:45 UTC (permalink / raw)
To: Marcelo Ricardo Leitner
Cc: network dev, linux-sctp, Vlad Yasevich, Neil Horman
On Wed, Jan 3, 2018 at 5:44 AM, Marcelo Ricardo Leitner
<marcelo.leitner@gmail.com> wrote:
> syzbot noticed a NULL pointer dereference panic in sctp_stream_free()
> which was caused by an incomplete error handling in sctp_stream_init().
> By not clearing stream->outcnt, it made a for() in sctp_stream_free()
> think that it had elements to free, but not, leading to the panic.
>
> As suggested by Xin Long, this patch also simplifies the error path by
> moving it to the only if() that uses it.
>
> See-also: https://www.spinics.net/lists/netdev/msg473756.html
> See-also: https://www.spinics.net/lists/netdev/msg465024.html
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Fixes: f952be79cebd ("sctp: introduce struct sctp_stream_out_ext")
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> net/sctp/stream.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index 76ea66be0bbee7d3f018676d52c8b95ba06dbcb1..524dfeb94c41ab1ac735746a8acf93af1c96ae48 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -156,9 +156,9 @@ int sctp_stream_init(struct sctp_stream *stream, __u16 outcnt, __u16 incnt,
> sctp_stream_outq_migrate(stream, NULL, outcnt);
> sched->sched_all(stream);
>
> - i = sctp_stream_alloc_out(stream, outcnt, gfp);
> - if (i)
> - return i;
> + ret = sctp_stream_alloc_out(stream, outcnt, gfp);
> + if (ret)
> + goto out;
>
> stream->outcnt = outcnt;
> for (i = 0; i < stream->outcnt; i++)
> @@ -170,19 +170,17 @@ int sctp_stream_init(struct sctp_stream *stream, __u16 outcnt, __u16 incnt,
> if (!incnt)
> goto out;
>
> - i = sctp_stream_alloc_in(stream, incnt, gfp);
> - if (i) {
> - ret = -ENOMEM;
> - goto free;
> + ret = sctp_stream_alloc_in(stream, incnt, gfp);
> + if (ret) {
> + sched->free(stream);
> + kfree(stream->out);
> + stream->out = NULL;
> + stream->outcnt = 0;
> + goto out;
> }
>
> stream->incnt = incnt;
> - goto out;
>
> -free:
> - sched->free(stream);
> - kfree(stream->out);
> - stream->out = NULL;
> out:
> return ret;
> }
> --
> 2.14.3
>
Reviewed-by: Xin Long <lucien.xin@gmail.com>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net] sctp: fix error path in sctp_stream_init
2018-01-02 21:44 [PATCH net] sctp: fix error path in sctp_stream_init Marcelo Ricardo Leitner
2018-01-03 6:45 ` Xin Long
@ 2018-01-03 10:32 ` Neil Horman
2018-01-03 16:30 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: Neil Horman @ 2018-01-03 10:32 UTC (permalink / raw)
To: Marcelo Ricardo Leitner; +Cc: netdev, linux-sctp, Xin Long, Vlad Yasevich
On Tue, Jan 02, 2018 at 07:44:37PM -0200, Marcelo Ricardo Leitner wrote:
> syzbot noticed a NULL pointer dereference panic in sctp_stream_free()
> which was caused by an incomplete error handling in sctp_stream_init().
> By not clearing stream->outcnt, it made a for() in sctp_stream_free()
> think that it had elements to free, but not, leading to the panic.
>
> As suggested by Xin Long, this patch also simplifies the error path by
> moving it to the only if() that uses it.
>
> See-also: https://www.spinics.net/lists/netdev/msg473756.html
> See-also: https://www.spinics.net/lists/netdev/msg465024.html
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Fixes: f952be79cebd ("sctp: introduce struct sctp_stream_out_ext")
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
> net/sctp/stream.c | 22 ++++++++++------------
> 1 file changed, 10 insertions(+), 12 deletions(-)
>
> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index 76ea66be0bbee7d3f018676d52c8b95ba06dbcb1..524dfeb94c41ab1ac735746a8acf93af1c96ae48 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -156,9 +156,9 @@ int sctp_stream_init(struct sctp_stream *stream, __u16 outcnt, __u16 incnt,
> sctp_stream_outq_migrate(stream, NULL, outcnt);
> sched->sched_all(stream);
>
> - i = sctp_stream_alloc_out(stream, outcnt, gfp);
> - if (i)
> - return i;
> + ret = sctp_stream_alloc_out(stream, outcnt, gfp);
> + if (ret)
> + goto out;
>
> stream->outcnt = outcnt;
> for (i = 0; i < stream->outcnt; i++)
> @@ -170,19 +170,17 @@ int sctp_stream_init(struct sctp_stream *stream, __u16 outcnt, __u16 incnt,
> if (!incnt)
> goto out;
>
> - i = sctp_stream_alloc_in(stream, incnt, gfp);
> - if (i) {
> - ret = -ENOMEM;
> - goto free;
> + ret = sctp_stream_alloc_in(stream, incnt, gfp);
> + if (ret) {
> + sched->free(stream);
> + kfree(stream->out);
> + stream->out = NULL;
> + stream->outcnt = 0;
> + goto out;
> }
>
> stream->incnt = incnt;
> - goto out;
>
> -free:
> - sched->free(stream);
> - kfree(stream->out);
> - stream->out = NULL;
> out:
> return ret;
> }
> --
> 2.14.3
>
>
Acked-by: Neil Horman <nhorman@tuxdriver.com>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH net] sctp: fix error path in sctp_stream_init
2018-01-02 21:44 [PATCH net] sctp: fix error path in sctp_stream_init Marcelo Ricardo Leitner
2018-01-03 6:45 ` Xin Long
2018-01-03 10:32 ` Neil Horman
@ 2018-01-03 16:30 ` David Miller
2 siblings, 0 replies; 4+ messages in thread
From: David Miller @ 2018-01-03 16:30 UTC (permalink / raw)
To: marcelo.leitner; +Cc: netdev, linux-sctp, lucien.xin, vyasevich, nhorman
From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Tue, 2 Jan 2018 19:44:37 -0200
> syzbot noticed a NULL pointer dereference panic in sctp_stream_free()
> which was caused by an incomplete error handling in sctp_stream_init().
> By not clearing stream->outcnt, it made a for() in sctp_stream_free()
> think that it had elements to free, but not, leading to the panic.
>
> As suggested by Xin Long, this patch also simplifies the error path by
> moving it to the only if() that uses it.
>
> See-also: https://www.spinics.net/lists/netdev/msg473756.html
> See-also: https://www.spinics.net/lists/netdev/msg465024.html
> Reported-by: syzbot <syzkaller@googlegroups.com>
> Fixes: f952be79cebd ("sctp: introduce struct sctp_stream_out_ext")
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Applied, thank you.
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2018-01-03 16:30 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-01-02 21:44 [PATCH net] sctp: fix error path in sctp_stream_init Marcelo Ricardo Leitner
2018-01-03 6:45 ` Xin Long
2018-01-03 10:32 ` Neil Horman
2018-01-03 16:30 ` David Miller
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox