All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] sctp: fix error handling on stream scheduler initialization
@ 2019-06-27 22:48 ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 6+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-06-27 22:48 UTC (permalink / raw)
  To: netdev; +Cc: Xin Long, Neil Horman, linux-sctp, Hillf Danton

It allocates the extended area for outbound streams only on sendmsg
calls, if they are not yet allocated.  When using the priority
stream scheduler, this initialization may imply into a subsequent
allocation, which may fail.  In this case, it was aborting the stream
scheduler initialization but leaving the ->ext pointer (allocated) in
there, thus in a partially initialized state.  On a subsequent call to
sendmsg, it would notice the ->ext pointer in there, and trip on
uninitialized stuff when trying to schedule the data chunk.

The fix is undo the ->ext initialization if the stream scheduler
initialization fails and avoid the partially initialized state.

Although syzkaller bisected this to commit 4ff40b86262b ("sctp: set
chunk transport correctly when it's a new asoc"), this bug was actually
introduced on the commit I marked below.

Reported-by: syzbot+c1a380d42b190ad1e559@syzkaller.appspotmail.com
Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations")
Tested-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/stream.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index 93ed07877337eace4ef7f4775dda5868359ada37..25946604af85c09917e63e5c4a8d7d6fa2caebc4 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -153,13 +153,20 @@ int sctp_stream_init(struct sctp_stream *stream, __u16 outcnt, __u16 incnt,
 int sctp_stream_init_ext(struct sctp_stream *stream, __u16 sid)
 {
 	struct sctp_stream_out_ext *soute;
+	int ret;
 
 	soute = kzalloc(sizeof(*soute), GFP_KERNEL);
 	if (!soute)
 		return -ENOMEM;
 	SCTP_SO(stream, sid)->ext = soute;
 
-	return sctp_sched_init_sid(stream, sid, GFP_KERNEL);
+	ret = sctp_sched_init_sid(stream, sid, GFP_KERNEL);
+	if (ret) {
+		kfree(SCTP_SO(stream, sid)->ext);
+		SCTP_SO(stream, sid)->ext = NULL;
+	}
+
+	return ret;
 }
 
 void sctp_stream_free(struct sctp_stream *stream)
-- 
2.21.0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH net] sctp: fix error handling on stream scheduler initialization
@ 2019-06-27 22:48 ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 6+ messages in thread
From: Marcelo Ricardo Leitner @ 2019-06-27 22:48 UTC (permalink / raw)
  To: netdev; +Cc: Xin Long, Neil Horman, linux-sctp, Hillf Danton

It allocates the extended area for outbound streams only on sendmsg
calls, if they are not yet allocated.  When using the priority
stream scheduler, this initialization may imply into a subsequent
allocation, which may fail.  In this case, it was aborting the stream
scheduler initialization but leaving the ->ext pointer (allocated) in
there, thus in a partially initialized state.  On a subsequent call to
sendmsg, it would notice the ->ext pointer in there, and trip on
uninitialized stuff when trying to schedule the data chunk.

The fix is undo the ->ext initialization if the stream scheduler
initialization fails and avoid the partially initialized state.

Although syzkaller bisected this to commit 4ff40b86262b ("sctp: set
chunk transport correctly when it's a new asoc"), this bug was actually
introduced on the commit I marked below.

Reported-by: syzbot+c1a380d42b190ad1e559@syzkaller.appspotmail.com
Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations")
Tested-by: Xin Long <lucien.xin@gmail.com>
Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
---
 net/sctp/stream.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/net/sctp/stream.c b/net/sctp/stream.c
index 93ed07877337eace4ef7f4775dda5868359ada37..25946604af85c09917e63e5c4a8d7d6fa2caebc4 100644
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -153,13 +153,20 @@ int sctp_stream_init(struct sctp_stream *stream, __u16 outcnt, __u16 incnt,
 int sctp_stream_init_ext(struct sctp_stream *stream, __u16 sid)
 {
 	struct sctp_stream_out_ext *soute;
+	int ret;
 
 	soute = kzalloc(sizeof(*soute), GFP_KERNEL);
 	if (!soute)
 		return -ENOMEM;
 	SCTP_SO(stream, sid)->ext = soute;
 
-	return sctp_sched_init_sid(stream, sid, GFP_KERNEL);
+	ret = sctp_sched_init_sid(stream, sid, GFP_KERNEL);
+	if (ret) {
+		kfree(SCTP_SO(stream, sid)->ext);
+		SCTP_SO(stream, sid)->ext = NULL;
+	}
+
+	return ret;
 }
 
 void sctp_stream_free(struct sctp_stream *stream)
-- 
2.21.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH net] sctp: fix error handling on stream scheduler initialization
  2019-06-27 22:48 ` Marcelo Ricardo Leitner
@ 2019-06-28 11:06   ` Neil Horman
  -1 siblings, 0 replies; 6+ messages in thread
From: Neil Horman @ 2019-06-28 11:06 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner; +Cc: netdev, Xin Long, linux-sctp, Hillf Danton

On Thu, Jun 27, 2019 at 07:48:10PM -0300, Marcelo Ricardo Leitner wrote:
> It allocates the extended area for outbound streams only on sendmsg
> calls, if they are not yet allocated.  When using the priority
> stream scheduler, this initialization may imply into a subsequent
> allocation, which may fail.  In this case, it was aborting the stream
> scheduler initialization but leaving the ->ext pointer (allocated) in
> there, thus in a partially initialized state.  On a subsequent call to
> sendmsg, it would notice the ->ext pointer in there, and trip on
> uninitialized stuff when trying to schedule the data chunk.
> 
> The fix is undo the ->ext initialization if the stream scheduler
> initialization fails and avoid the partially initialized state.
> 
> Although syzkaller bisected this to commit 4ff40b86262b ("sctp: set
> chunk transport correctly when it's a new asoc"), this bug was actually
> introduced on the commit I marked below.
> 
> Reported-by: syzbot+c1a380d42b190ad1e559@syzkaller.appspotmail.com
> Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations")
> Tested-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
>  net/sctp/stream.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index 93ed07877337eace4ef7f4775dda5868359ada37..25946604af85c09917e63e5c4a8d7d6fa2caebc4 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -153,13 +153,20 @@ int sctp_stream_init(struct sctp_stream *stream, __u16 outcnt, __u16 incnt,
>  int sctp_stream_init_ext(struct sctp_stream *stream, __u16 sid)
>  {
>  	struct sctp_stream_out_ext *soute;
> +	int ret;
>  
>  	soute = kzalloc(sizeof(*soute), GFP_KERNEL);
>  	if (!soute)
>  		return -ENOMEM;
>  	SCTP_SO(stream, sid)->ext = soute;
>  
> -	return sctp_sched_init_sid(stream, sid, GFP_KERNEL);
> +	ret = sctp_sched_init_sid(stream, sid, GFP_KERNEL);
> +	if (ret) {
> +		kfree(SCTP_SO(stream, sid)->ext);
> +		SCTP_SO(stream, sid)->ext = NULL;
> +	}
> +
> +	return ret;
>  }
>  
>  void sctp_stream_free(struct sctp_stream *stream)
> -- 
> 2.21.0
> 
> 
Acked-by: Neil Horman <nhorman@redhat.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] sctp: fix error handling on stream scheduler initialization
@ 2019-06-28 11:06   ` Neil Horman
  0 siblings, 0 replies; 6+ messages in thread
From: Neil Horman @ 2019-06-28 11:06 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner; +Cc: netdev, Xin Long, linux-sctp, Hillf Danton

On Thu, Jun 27, 2019 at 07:48:10PM -0300, Marcelo Ricardo Leitner wrote:
> It allocates the extended area for outbound streams only on sendmsg
> calls, if they are not yet allocated.  When using the priority
> stream scheduler, this initialization may imply into a subsequent
> allocation, which may fail.  In this case, it was aborting the stream
> scheduler initialization but leaving the ->ext pointer (allocated) in
> there, thus in a partially initialized state.  On a subsequent call to
> sendmsg, it would notice the ->ext pointer in there, and trip on
> uninitialized stuff when trying to schedule the data chunk.
> 
> The fix is undo the ->ext initialization if the stream scheduler
> initialization fails and avoid the partially initialized state.
> 
> Although syzkaller bisected this to commit 4ff40b86262b ("sctp: set
> chunk transport correctly when it's a new asoc"), this bug was actually
> introduced on the commit I marked below.
> 
> Reported-by: syzbot+c1a380d42b190ad1e559@syzkaller.appspotmail.com
> Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations")
> Tested-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
> ---
>  net/sctp/stream.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/net/sctp/stream.c b/net/sctp/stream.c
> index 93ed07877337eace4ef7f4775dda5868359ada37..25946604af85c09917e63e5c4a8d7d6fa2caebc4 100644
> --- a/net/sctp/stream.c
> +++ b/net/sctp/stream.c
> @@ -153,13 +153,20 @@ int sctp_stream_init(struct sctp_stream *stream, __u16 outcnt, __u16 incnt,
>  int sctp_stream_init_ext(struct sctp_stream *stream, __u16 sid)
>  {
>  	struct sctp_stream_out_ext *soute;
> +	int ret;
>  
>  	soute = kzalloc(sizeof(*soute), GFP_KERNEL);
>  	if (!soute)
>  		return -ENOMEM;
>  	SCTP_SO(stream, sid)->ext = soute;
>  
> -	return sctp_sched_init_sid(stream, sid, GFP_KERNEL);
> +	ret = sctp_sched_init_sid(stream, sid, GFP_KERNEL);
> +	if (ret) {
> +		kfree(SCTP_SO(stream, sid)->ext);
> +		SCTP_SO(stream, sid)->ext = NULL;
> +	}
> +
> +	return ret;
>  }
>  
>  void sctp_stream_free(struct sctp_stream *stream)
> -- 
> 2.21.0
> 
> 
Acked-by: Neil Horman <nhorman@redhat.com>

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] sctp: fix error handling on stream scheduler initialization
  2019-06-27 22:48 ` Marcelo Ricardo Leitner
@ 2019-07-02  2:02   ` David Miller
  -1 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2019-07-02  2:02 UTC (permalink / raw)
  To: marcelo.leitner; +Cc: netdev, lucien.xin, nhorman, linux-sctp, hdanton

From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Thu, 27 Jun 2019 19:48:10 -0300

> It allocates the extended area for outbound streams only on sendmsg
> calls, if they are not yet allocated.  When using the priority
> stream scheduler, this initialization may imply into a subsequent
> allocation, which may fail.  In this case, it was aborting the stream
> scheduler initialization but leaving the ->ext pointer (allocated) in
> there, thus in a partially initialized state.  On a subsequent call to
> sendmsg, it would notice the ->ext pointer in there, and trip on
> uninitialized stuff when trying to schedule the data chunk.
> 
> The fix is undo the ->ext initialization if the stream scheduler
> initialization fails and avoid the partially initialized state.
> 
> Although syzkaller bisected this to commit 4ff40b86262b ("sctp: set
> chunk transport correctly when it's a new asoc"), this bug was actually
> introduced on the commit I marked below.
> 
> Reported-by: syzbot+c1a380d42b190ad1e559@syzkaller.appspotmail.com
> Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations")
> Tested-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [PATCH net] sctp: fix error handling on stream scheduler initialization
@ 2019-07-02  2:02   ` David Miller
  0 siblings, 0 replies; 6+ messages in thread
From: David Miller @ 2019-07-02  2:02 UTC (permalink / raw)
  To: marcelo.leitner; +Cc: netdev, lucien.xin, nhorman, linux-sctp, hdanton

From: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>
Date: Thu, 27 Jun 2019 19:48:10 -0300

> It allocates the extended area for outbound streams only on sendmsg
> calls, if they are not yet allocated.  When using the priority
> stream scheduler, this initialization may imply into a subsequent
> allocation, which may fail.  In this case, it was aborting the stream
> scheduler initialization but leaving the ->ext pointer (allocated) in
> there, thus in a partially initialized state.  On a subsequent call to
> sendmsg, it would notice the ->ext pointer in there, and trip on
> uninitialized stuff when trying to schedule the data chunk.
> 
> The fix is undo the ->ext initialization if the stream scheduler
> initialization fails and avoid the partially initialized state.
> 
> Although syzkaller bisected this to commit 4ff40b86262b ("sctp: set
> chunk transport correctly when it's a new asoc"), this bug was actually
> introduced on the commit I marked below.
> 
> Reported-by: syzbot+c1a380d42b190ad1e559@syzkaller.appspotmail.com
> Fixes: 5bbbbe32a431 ("sctp: introduce stream scheduler foundations")
> Tested-by: Xin Long <lucien.xin@gmail.com>
> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

Applied and queued up for -stable, thanks.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2019-07-02  2:02 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-27 22:48 [PATCH net] sctp: fix error handling on stream scheduler initialization Marcelo Ricardo Leitner
2019-06-27 22:48 ` Marcelo Ricardo Leitner
2019-06-28 11:06 ` Neil Horman
2019-06-28 11:06   ` Neil Horman
2019-07-02  2:02 ` David Miller
2019-07-02  2:02   ` David Miller

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.