* [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