All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] sctp: change sk state only when it has assocs in sctp_shutdown
@ 2016-11-13 13:44 ` Xin Long
  0 siblings, 0 replies; 8+ messages in thread
From: Xin Long @ 2016-11-13 13:44 UTC (permalink / raw)
  To: network dev, linux-sctp
  Cc: davem, Marcelo Ricardo Leitner, Neil Horman, Vlad Yasevich, andreyknvl

Now when users shutdown a sock with SEND_SHUTDOWN in sctp, even if
this sock has no connection (assoc), sk state would be changed to
SCTP_SS_CLOSING, which is not as we expect.

Besides, after that if users try to listen on this sock, kernel
could even panic when it dereference sctp_sk(sk)->bind_hash in
sctp_inet_listen, as bind_hash is null when sock has no assoc.

This patch is to move sk state change after checking sk assocs
is not empty, and also merge these two if() conditions and reduce
indent level.

Fixes: d46e416c11c8 ("sctp: sctp should change socket state when shutdown is received")
Reported-by: Andrey Konovalov <andreyknvl@google.com>
Tested-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/socket.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index faa48ff..f23ad91 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4285,19 +4285,18 @@ static void sctp_shutdown(struct sock *sk, int how)
 {
 	struct net *net = sock_net(sk);
 	struct sctp_endpoint *ep;
-	struct sctp_association *asoc;
 
 	if (!sctp_style(sk, TCP))
 		return;
 
-	if (how & SEND_SHUTDOWN) {
+	ep = sctp_sk(sk)->ep;
+	if (how & SEND_SHUTDOWN && !list_empty(&ep->asocs)) {
+		struct sctp_association *asoc;
+
 		sk->sk_state = SCTP_SS_CLOSING;
-		ep = sctp_sk(sk)->ep;
-		if (!list_empty(&ep->asocs)) {
-			asoc = list_entry(ep->asocs.next,
-					  struct sctp_association, asocs);
-			sctp_primitive_SHUTDOWN(net, asoc, NULL);
-		}
+		asoc = list_entry(ep->asocs.next,
+				  struct sctp_association, asocs);
+		sctp_primitive_SHUTDOWN(net, asoc, NULL);
 	}
 }
 
-- 
2.1.0

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

* [PATCH net] sctp: change sk state only when it has assocs in sctp_shutdown
@ 2016-11-13 13:44 ` Xin Long
  0 siblings, 0 replies; 8+ messages in thread
From: Xin Long @ 2016-11-13 13:44 UTC (permalink / raw)
  To: network dev, linux-sctp
  Cc: davem, Marcelo Ricardo Leitner, Neil Horman, Vlad Yasevich, andreyknvl

Now when users shutdown a sock with SEND_SHUTDOWN in sctp, even if
this sock has no connection (assoc), sk state would be changed to
SCTP_SS_CLOSING, which is not as we expect.

Besides, after that if users try to listen on this sock, kernel
could even panic when it dereference sctp_sk(sk)->bind_hash in
sctp_inet_listen, as bind_hash is null when sock has no assoc.

This patch is to move sk state change after checking sk assocs
is not empty, and also merge these two if() conditions and reduce
indent level.

Fixes: d46e416c11c8 ("sctp: sctp should change socket state when shutdown is received")
Reported-by: Andrey Konovalov <andreyknvl@google.com>
Tested-by: Andrey Konovalov <andreyknvl@google.com>
Signed-off-by: Xin Long <lucien.xin@gmail.com>
---
 net/sctp/socket.c | 15 +++++++--------
 1 file changed, 7 insertions(+), 8 deletions(-)

diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index faa48ff..f23ad91 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -4285,19 +4285,18 @@ static void sctp_shutdown(struct sock *sk, int how)
 {
 	struct net *net = sock_net(sk);
 	struct sctp_endpoint *ep;
-	struct sctp_association *asoc;
 
 	if (!sctp_style(sk, TCP))
 		return;
 
-	if (how & SEND_SHUTDOWN) {
+	ep = sctp_sk(sk)->ep;
+	if (how & SEND_SHUTDOWN && !list_empty(&ep->asocs)) {
+		struct sctp_association *asoc;
+
 		sk->sk_state = SCTP_SS_CLOSING;
-		ep = sctp_sk(sk)->ep;
-		if (!list_empty(&ep->asocs)) {
-			asoc = list_entry(ep->asocs.next,
-					  struct sctp_association, asocs);
-			sctp_primitive_SHUTDOWN(net, asoc, NULL);
-		}
+		asoc = list_entry(ep->asocs.next,
+				  struct sctp_association, asocs);
+		sctp_primitive_SHUTDOWN(net, asoc, NULL);
 	}
 }
 
-- 
2.1.0


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

* Re: [PATCH net] sctp: change sk state only when it has assocs in sctp_shutdown
  2016-11-13 13:44 ` Xin Long
@ 2016-11-14 10:32   ` Marcelo Ricardo Leitner
  -1 siblings, 0 replies; 8+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-11-14 10:32 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, linux-sctp, davem, Neil Horman, Vlad Yasevich, andreyknvl

On Sun, Nov 13, 2016 at 09:44:37PM +0800, Xin Long wrote:
> Now when users shutdown a sock with SEND_SHUTDOWN in sctp, even if
> this sock has no connection (assoc), sk state would be changed to
> SCTP_SS_CLOSING, which is not as we expect.
> 
> Besides, after that if users try to listen on this sock, kernel
> could even panic when it dereference sctp_sk(sk)->bind_hash in
> sctp_inet_listen, as bind_hash is null when sock has no assoc.
> 
> This patch is to move sk state change after checking sk assocs
> is not empty, and also merge these two if() conditions and reduce
> indent level.
> 
> Fixes: d46e416c11c8 ("sctp: sctp should change socket state when shutdown is received")
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Tested-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  net/sctp/socket.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index faa48ff..f23ad91 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4285,19 +4285,18 @@ static void sctp_shutdown(struct sock *sk, int how)
>  {
>  	struct net *net = sock_net(sk);
>  	struct sctp_endpoint *ep;
> -	struct sctp_association *asoc;
>  
>  	if (!sctp_style(sk, TCP))
>  		return;
>  
> -	if (how & SEND_SHUTDOWN) {
> +	ep = sctp_sk(sk)->ep;
> +	if (how & SEND_SHUTDOWN && !list_empty(&ep->asocs)) {
> +		struct sctp_association *asoc;
> +
>  		sk->sk_state = SCTP_SS_CLOSING;
> -		ep = sctp_sk(sk)->ep;
> -		if (!list_empty(&ep->asocs)) {
> -			asoc = list_entry(ep->asocs.next,
> -					  struct sctp_association, asocs);
> -			sctp_primitive_SHUTDOWN(net, asoc, NULL);
> -		}
> +		asoc = list_entry(ep->asocs.next,
> +				  struct sctp_association, asocs);
> +		sctp_primitive_SHUTDOWN(net, asoc, NULL);
>  	}
>  }
>  
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* Re: [PATCH net] sctp: change sk state only when it has assocs in sctp_shutdown
@ 2016-11-14 10:32   ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 8+ messages in thread
From: Marcelo Ricardo Leitner @ 2016-11-14 10:32 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, linux-sctp, davem, Neil Horman, Vlad Yasevich, andreyknvl

On Sun, Nov 13, 2016 at 09:44:37PM +0800, Xin Long wrote:
> Now when users shutdown a sock with SEND_SHUTDOWN in sctp, even if
> this sock has no connection (assoc), sk state would be changed to
> SCTP_SS_CLOSING, which is not as we expect.
> 
> Besides, after that if users try to listen on this sock, kernel
> could even panic when it dereference sctp_sk(sk)->bind_hash in
> sctp_inet_listen, as bind_hash is null when sock has no assoc.
> 
> This patch is to move sk state change after checking sk assocs
> is not empty, and also merge these two if() conditions and reduce
> indent level.
> 
> Fixes: d46e416c11c8 ("sctp: sctp should change socket state when shutdown is received")
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Tested-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@gmail.com>

> ---
>  net/sctp/socket.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index faa48ff..f23ad91 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4285,19 +4285,18 @@ static void sctp_shutdown(struct sock *sk, int how)
>  {
>  	struct net *net = sock_net(sk);
>  	struct sctp_endpoint *ep;
> -	struct sctp_association *asoc;
>  
>  	if (!sctp_style(sk, TCP))
>  		return;
>  
> -	if (how & SEND_SHUTDOWN) {
> +	ep = sctp_sk(sk)->ep;
> +	if (how & SEND_SHUTDOWN && !list_empty(&ep->asocs)) {
> +		struct sctp_association *asoc;
> +
>  		sk->sk_state = SCTP_SS_CLOSING;
> -		ep = sctp_sk(sk)->ep;
> -		if (!list_empty(&ep->asocs)) {
> -			asoc = list_entry(ep->asocs.next,
> -					  struct sctp_association, asocs);
> -			sctp_primitive_SHUTDOWN(net, asoc, NULL);
> -		}
> +		asoc = list_entry(ep->asocs.next,
> +				  struct sctp_association, asocs);
> +		sctp_primitive_SHUTDOWN(net, asoc, NULL);
>  	}
>  }
>  
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

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

* Re: [PATCH net] sctp: change sk state only when it has assocs in sctp_shutdown
  2016-11-13 13:44 ` Xin Long
@ 2016-11-14 15:52   ` Neil Horman
  -1 siblings, 0 replies; 8+ messages in thread
From: Neil Horman @ 2016-11-14 15:52 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner,
	Vlad Yasevich, andreyknvl

On Sun, Nov 13, 2016 at 09:44:37PM +0800, Xin Long wrote:
> Now when users shutdown a sock with SEND_SHUTDOWN in sctp, even if
> this sock has no connection (assoc), sk state would be changed to
> SCTP_SS_CLOSING, which is not as we expect.
> 
> Besides, after that if users try to listen on this sock, kernel
> could even panic when it dereference sctp_sk(sk)->bind_hash in
> sctp_inet_listen, as bind_hash is null when sock has no assoc.
> 
> This patch is to move sk state change after checking sk assocs
> is not empty, and also merge these two if() conditions and reduce
> indent level.
> 
> Fixes: d46e416c11c8 ("sctp: sctp should change socket state when shutdown is received")
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Tested-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/sctp/socket.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index faa48ff..f23ad91 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4285,19 +4285,18 @@ static void sctp_shutdown(struct sock *sk, int how)
>  {
>  	struct net *net = sock_net(sk);
>  	struct sctp_endpoint *ep;
> -	struct sctp_association *asoc;
>  
>  	if (!sctp_style(sk, TCP))
>  		return;
>  
> -	if (how & SEND_SHUTDOWN) {
> +	ep = sctp_sk(sk)->ep;
> +	if (how & SEND_SHUTDOWN && !list_empty(&ep->asocs)) {
> +		struct sctp_association *asoc;
> +
>  		sk->sk_state = SCTP_SS_CLOSING;
> -		ep = sctp_sk(sk)->ep;
> -		if (!list_empty(&ep->asocs)) {
> -			asoc = list_entry(ep->asocs.next,
> -					  struct sctp_association, asocs);
> -			sctp_primitive_SHUTDOWN(net, asoc, NULL);
> -		}
> +		asoc = list_entry(ep->asocs.next,
> +				  struct sctp_association, asocs);
> +		sctp_primitive_SHUTDOWN(net, asoc, NULL);
>  	}
>  }
>  
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>

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

* Re: [PATCH net] sctp: change sk state only when it has assocs in sctp_shutdown
@ 2016-11-14 15:52   ` Neil Horman
  0 siblings, 0 replies; 8+ messages in thread
From: Neil Horman @ 2016-11-14 15:52 UTC (permalink / raw)
  To: Xin Long
  Cc: network dev, linux-sctp, davem, Marcelo Ricardo Leitner,
	Vlad Yasevich, andreyknvl

On Sun, Nov 13, 2016 at 09:44:37PM +0800, Xin Long wrote:
> Now when users shutdown a sock with SEND_SHUTDOWN in sctp, even if
> this sock has no connection (assoc), sk state would be changed to
> SCTP_SS_CLOSING, which is not as we expect.
> 
> Besides, after that if users try to listen on this sock, kernel
> could even panic when it dereference sctp_sk(sk)->bind_hash in
> sctp_inet_listen, as bind_hash is null when sock has no assoc.
> 
> This patch is to move sk state change after checking sk assocs
> is not empty, and also merge these two if() conditions and reduce
> indent level.
> 
> Fixes: d46e416c11c8 ("sctp: sctp should change socket state when shutdown is received")
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Tested-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>
> ---
>  net/sctp/socket.c | 15 +++++++--------
>  1 file changed, 7 insertions(+), 8 deletions(-)
> 
> diff --git a/net/sctp/socket.c b/net/sctp/socket.c
> index faa48ff..f23ad91 100644
> --- a/net/sctp/socket.c
> +++ b/net/sctp/socket.c
> @@ -4285,19 +4285,18 @@ static void sctp_shutdown(struct sock *sk, int how)
>  {
>  	struct net *net = sock_net(sk);
>  	struct sctp_endpoint *ep;
> -	struct sctp_association *asoc;
>  
>  	if (!sctp_style(sk, TCP))
>  		return;
>  
> -	if (how & SEND_SHUTDOWN) {
> +	ep = sctp_sk(sk)->ep;
> +	if (how & SEND_SHUTDOWN && !list_empty(&ep->asocs)) {
> +		struct sctp_association *asoc;
> +
>  		sk->sk_state = SCTP_SS_CLOSING;
> -		ep = sctp_sk(sk)->ep;
> -		if (!list_empty(&ep->asocs)) {
> -			asoc = list_entry(ep->asocs.next,
> -					  struct sctp_association, asocs);
> -			sctp_primitive_SHUTDOWN(net, asoc, NULL);
> -		}
> +		asoc = list_entry(ep->asocs.next,
> +				  struct sctp_association, asocs);
> +		sctp_primitive_SHUTDOWN(net, asoc, NULL);
>  	}
>  }
>  
> -- 
> 2.1.0
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Acked-by: Neil Horman <nhorman@tuxdriver.com>


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

* Re: [PATCH net] sctp: change sk state only when it has assocs in sctp_shutdown
  2016-11-13 13:44 ` Xin Long
@ 2016-11-14 21:23   ` David Miller
  -1 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2016-11-14 21:23 UTC (permalink / raw)
  To: lucien.xin
  Cc: netdev, linux-sctp, marcelo.leitner, nhorman, vyasevich, andreyknvl

From: Xin Long <lucien.xin@gmail.com>
Date: Sun, 13 Nov 2016 21:44:37 +0800

> Now when users shutdown a sock with SEND_SHUTDOWN in sctp, even if
> this sock has no connection (assoc), sk state would be changed to
> SCTP_SS_CLOSING, which is not as we expect.
> 
> Besides, after that if users try to listen on this sock, kernel
> could even panic when it dereference sctp_sk(sk)->bind_hash in
> sctp_inet_listen, as bind_hash is null when sock has no assoc.
> 
> This patch is to move sk state change after checking sk assocs
> is not empty, and also merge these two if() conditions and reduce
> indent level.
> 
> Fixes: d46e416c11c8 ("sctp: sctp should change socket state when shutdown is received")
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Tested-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Applied and queued up for -stable, thanks.

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

* Re: [PATCH net] sctp: change sk state only when it has assocs in sctp_shutdown
@ 2016-11-14 21:23   ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2016-11-14 21:23 UTC (permalink / raw)
  To: lucien.xin
  Cc: netdev, linux-sctp, marcelo.leitner, nhorman, vyasevich, andreyknvl

From: Xin Long <lucien.xin@gmail.com>
Date: Sun, 13 Nov 2016 21:44:37 +0800

> Now when users shutdown a sock with SEND_SHUTDOWN in sctp, even if
> this sock has no connection (assoc), sk state would be changed to
> SCTP_SS_CLOSING, which is not as we expect.
> 
> Besides, after that if users try to listen on this sock, kernel
> could even panic when it dereference sctp_sk(sk)->bind_hash in
> sctp_inet_listen, as bind_hash is null when sock has no assoc.
> 
> This patch is to move sk state change after checking sk assocs
> is not empty, and also merge these two if() conditions and reduce
> indent level.
> 
> Fixes: d46e416c11c8 ("sctp: sctp should change socket state when shutdown is received")
> Reported-by: Andrey Konovalov <andreyknvl@google.com>
> Tested-by: Andrey Konovalov <andreyknvl@google.com>
> Signed-off-by: Xin Long <lucien.xin@gmail.com>

Applied and queued up for -stable, thanks.

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

end of thread, other threads:[~2016-11-14 21:23 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-13 13:44 [PATCH net] sctp: change sk state only when it has assocs in sctp_shutdown Xin Long
2016-11-13 13:44 ` Xin Long
2016-11-14 10:32 ` Marcelo Ricardo Leitner
2016-11-14 10:32   ` Marcelo Ricardo Leitner
2016-11-14 15:52 ` Neil Horman
2016-11-14 15:52   ` Neil Horman
2016-11-14 21:23 ` David Miller
2016-11-14 21:23   ` 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.