All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket is closed
@ 2020-05-20 15:15 ` Jere Leppänen
  0 siblings, 0 replies; 8+ messages in thread
From: Jere Leppänen @ 2020-05-20 15:15 UTC (permalink / raw)
  To: linux-sctp, netdev, Vlad Yasevich, Neil Horman,
	Marcelo Ricardo Leitner, David S . Miller
  Cc: David Laight

Commit bdf6fa52f01b ("sctp: handle association restarts when the
socket is closed.") starts shutdown when an association is restarted,
if in SHUTDOWN-PENDING state and the socket is closed. However, the
rationale stated in that commit applies also when in SHUTDOWN-SENT
state - we don't want to move an association to ESTABLISHED state when
the socket has been closed, because that results in an association
that is unreachable from user space.

The problem scenario:

1.  Client crashes and/or restarts.

2.  Server (using one-to-one socket) calls close(). SHUTDOWN is lost.

3.  Client reconnects using the same addresses and ports.

4.  Server's association is restarted. The association and the socket
    move to ESTABLISHED state, even though the server process has
    closed its descriptor.

Also, after step 4 when the server process exits, some resources are
leaked in an attempt to release the underlying inet sock structure in
ESTABLISHED state:

    IPv4: Attempt to release TCP socket in state 1 00000000377288c7

Fix by acting the same way as in SHUTDOWN-PENDING state. That is, if
an association is restarted in SHUTDOWN-SENT state and the socket is
closed, then start shutdown and don't move the association or the
socket to ESTABLISHED state.

Fixes: bdf6fa52f01b ("sctp: handle association restarts when the socket is closed.")
Signed-off-by: Jere Leppänen <jere.leppanen@nokia.com>
---
 net/sctp/sm_statefuns.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index 26788f4a3b9e..e86620fbd90f 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -1856,12 +1856,13 @@ static enum sctp_disposition sctp_sf_do_dupcook_a(
 	/* Update the content of current association. */
 	sctp_add_cmd_sf(commands, SCTP_CMD_UPDATE_ASSOC, SCTP_ASOC(new_asoc));
 	sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP, SCTP_ULPEVENT(ev));
-	if (sctp_state(asoc, SHUTDOWN_PENDING) &&
+	if ((sctp_state(asoc, SHUTDOWN_PENDING) ||
+	     sctp_state(asoc, SHUTDOWN_SENT)) &&
 	    (sctp_sstate(asoc->base.sk, CLOSING) ||
 	     sock_flag(asoc->base.sk, SOCK_DEAD))) {
-		/* if were currently in SHUTDOWN_PENDING, but the socket
-		 * has been closed by user, don't transition to ESTABLISHED.
-		 * Instead trigger SHUTDOWN bundled with COOKIE_ACK.
+		/* If the socket has been closed by user, don't
+		 * transition to ESTABLISHED. Instead trigger SHUTDOWN
+		 * bundled with COOKIE_ACK.
 		 */
 		sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(repl));
 		return sctp_sf_do_9_2_start_shutdown(net, ep, asoc,

base-commit: 20a785aa52c82246055a089e55df9dac47d67da1
-- 
2.25.2


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

* [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket is
@ 2020-05-20 15:15 ` Jere Leppänen
  0 siblings, 0 replies; 8+ messages in thread
From: Jere Leppänen @ 2020-05-20 15:15 UTC (permalink / raw)
  To: linux-sctp, netdev, Vlad Yasevich, Neil Horman,
	Marcelo Ricardo Leitner, David S . Miller
  Cc: David Laight

Q29tbWl0IGJkZjZmYTUyZjAxYiAoInNjdHA6IGhhbmRsZSBhc3NvY2lhdGlvbiByZXN0YXJ0cyB3
aGVuIHRoZQpzb2NrZXQgaXMgY2xvc2VkLiIpIHN0YXJ0cyBzaHV0ZG93biB3aGVuIGFuIGFzc29j
aWF0aW9uIGlzIHJlc3RhcnRlZCwKaWYgaW4gU0hVVERPV04tUEVORElORyBzdGF0ZSBhbmQgdGhl
IHNvY2tldCBpcyBjbG9zZWQuIEhvd2V2ZXIsIHRoZQpyYXRpb25hbGUgc3RhdGVkIGluIHRoYXQg
Y29tbWl0IGFwcGxpZXMgYWxzbyB3aGVuIGluIFNIVVRET1dOLVNFTlQKc3RhdGUgLSB3ZSBkb24n
dCB3YW50IHRvIG1vdmUgYW4gYXNzb2NpYXRpb24gdG8gRVNUQUJMSVNIRUQgc3RhdGUgd2hlbgp0
aGUgc29ja2V0IGhhcyBiZWVuIGNsb3NlZCwgYmVjYXVzZSB0aGF0IHJlc3VsdHMgaW4gYW4gYXNz
b2NpYXRpb24KdGhhdCBpcyB1bnJlYWNoYWJsZSBmcm9tIHVzZXIgc3BhY2UuCgpUaGUgcHJvYmxl
bSBzY2VuYXJpbzoKCjEuICBDbGllbnQgY3Jhc2hlcyBhbmQvb3IgcmVzdGFydHMuCgoyLiAgU2Vy
dmVyICh1c2luZyBvbmUtdG8tb25lIHNvY2tldCkgY2FsbHMgY2xvc2UoKS4gU0hVVERPV04gaXMg
bG9zdC4KCjMuICBDbGllbnQgcmVjb25uZWN0cyB1c2luZyB0aGUgc2FtZSBhZGRyZXNzZXMgYW5k
IHBvcnRzLgoKNC4gIFNlcnZlcidzIGFzc29jaWF0aW9uIGlzIHJlc3RhcnRlZC4gVGhlIGFzc29j
aWF0aW9uIGFuZCB0aGUgc29ja2V0CiAgICBtb3ZlIHRvIEVTVEFCTElTSEVEIHN0YXRlLCBldmVu
IHRob3VnaCB0aGUgc2VydmVyIHByb2Nlc3MgaGFzCiAgICBjbG9zZWQgaXRzIGRlc2NyaXB0b3Iu
CgpBbHNvLCBhZnRlciBzdGVwIDQgd2hlbiB0aGUgc2VydmVyIHByb2Nlc3MgZXhpdHMsIHNvbWUg
cmVzb3VyY2VzIGFyZQpsZWFrZWQgaW4gYW4gYXR0ZW1wdCB0byByZWxlYXNlIHRoZSB1bmRlcmx5
aW5nIGluZXQgc29jayBzdHJ1Y3R1cmUgaW4KRVNUQUJMSVNIRUQgc3RhdGU6CgogICAgSVB2NDog
QXR0ZW1wdCB0byByZWxlYXNlIFRDUCBzb2NrZXQgaW4gc3RhdGUgMSAwMDAwMDAwMDM3NzI4OGM3
CgpGaXggYnkgYWN0aW5nIHRoZSBzYW1lIHdheSBhcyBpbiBTSFVURE9XTi1QRU5ESU5HIHN0YXRl
LiBUaGF0IGlzLCBpZgphbiBhc3NvY2lhdGlvbiBpcyByZXN0YXJ0ZWQgaW4gU0hVVERPV04tU0VO
VCBzdGF0ZSBhbmQgdGhlIHNvY2tldCBpcwpjbG9zZWQsIHRoZW4gc3RhcnQgc2h1dGRvd24gYW5k
IGRvbid0IG1vdmUgdGhlIGFzc29jaWF0aW9uIG9yIHRoZQpzb2NrZXQgdG8gRVNUQUJMSVNIRUQg
c3RhdGUuCgpGaXhlczogYmRmNmZhNTJmMDFiICgic2N0cDogaGFuZGxlIGFzc29jaWF0aW9uIHJl
c3RhcnRzIHdoZW4gdGhlIHNvY2tldCBpcyBjbG9zZWQuIikKU2lnbmVkLW9mZi1ieTogSmVyZSBM
ZXBww6RuZW4gPGplcmUubGVwcGFuZW5Abm9raWEuY29tPgotLS0KIG5ldC9zY3RwL3NtX3N0YXRl
ZnVucy5jIHwgOSArKysrKy0tLS0KIDEgZmlsZSBjaGFuZ2VkLCA1IGluc2VydGlvbnMoKyksIDQg
ZGVsZXRpb25zKC0pCgpkaWZmIC0tZ2l0IGEvbmV0L3NjdHAvc21fc3RhdGVmdW5zLmMgYi9uZXQv
c2N0cC9zbV9zdGF0ZWZ1bnMuYwppbmRleCAyNjc4OGY0YTNiOWUuLmU4NjYyMGZiZDkwZiAxMDA2
NDQKLS0tIGEvbmV0L3NjdHAvc21fc3RhdGVmdW5zLmMKKysrIGIvbmV0L3NjdHAvc21fc3RhdGVm
dW5zLmMKQEAgLTE4NTYsMTIgKzE4NTYsMTMgQEAgc3RhdGljIGVudW0gc2N0cF9kaXNwb3NpdGlv
biBzY3RwX3NmX2RvX2R1cGNvb2tfYSgKIAkvKiBVcGRhdGUgdGhlIGNvbnRlbnQgb2YgY3VycmVu
dCBhc3NvY2lhdGlvbi4gKi8KIAlzY3RwX2FkZF9jbWRfc2YoY29tbWFuZHMsIFNDVFBfQ01EX1VQ
REFURV9BU1NPQywgU0NUUF9BU09DKG5ld19hc29jKSk7CiAJc2N0cF9hZGRfY21kX3NmKGNvbW1h
bmRzLCBTQ1RQX0NNRF9FVkVOVF9VTFAsIFNDVFBfVUxQRVZFTlQoZXYpKTsKLQlpZiAoc2N0cF9z
dGF0ZShhc29jLCBTSFVURE9XTl9QRU5ESU5HKSAmJgorCWlmICgoc2N0cF9zdGF0ZShhc29jLCBT
SFVURE9XTl9QRU5ESU5HKSB8fAorCSAgICAgc2N0cF9zdGF0ZShhc29jLCBTSFVURE9XTl9TRU5U
KSkgJiYKIAkgICAgKHNjdHBfc3N0YXRlKGFzb2MtPmJhc2Uuc2ssIENMT1NJTkcpIHx8CiAJICAg
ICBzb2NrX2ZsYWcoYXNvYy0+YmFzZS5zaywgU09DS19ERUFEKSkpIHsKLQkJLyogaWYgd2VyZSBj
dXJyZW50bHkgaW4gU0hVVERPV05fUEVORElORywgYnV0IHRoZSBzb2NrZXQKLQkJICogaGFzIGJl
ZW4gY2xvc2VkIGJ5IHVzZXIsIGRvbid0IHRyYW5zaXRpb24gdG8gRVNUQUJMSVNIRUQuCi0JCSAq
IEluc3RlYWQgdHJpZ2dlciBTSFVURE9XTiBidW5kbGVkIHdpdGggQ09PS0lFX0FDSy4KKwkJLyog
SWYgdGhlIHNvY2tldCBoYXMgYmVlbiBjbG9zZWQgYnkgdXNlciwgZG9uJ3QKKwkJICogdHJhbnNp
dGlvbiB0byBFU1RBQkxJU0hFRC4gSW5zdGVhZCB0cmlnZ2VyIFNIVVRET1dOCisJCSAqIGJ1bmRs
ZWQgd2l0aCBDT09LSUVfQUNLLgogCQkgKi8KIAkJc2N0cF9hZGRfY21kX3NmKGNvbW1hbmRzLCBT
Q1RQX0NNRF9SRVBMWSwgU0NUUF9DSFVOSyhyZXBsKSk7CiAJCXJldHVybiBzY3RwX3NmX2RvXzlf
Ml9zdGFydF9zaHV0ZG93bihuZXQsIGVwLCBhc29jLAoKYmFzZS1jb21taXQ6IDIwYTc4NWFhNTJj
ODIyNDYwNTVhMDg5ZTU1ZGY5ZGFjNDdkNjdkYTEKLS0gCjIuMjUuMgoK

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

* Re: [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket is closed
  2020-05-20 15:15 ` [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket is Jere Leppänen
@ 2020-05-20 19:46   ` Marcelo Ricardo Leitner
  -1 siblings, 0 replies; 8+ messages in thread
From: Marcelo Ricardo Leitner @ 2020-05-20 19:46 UTC (permalink / raw)
  To: Jere Leppänen
  Cc: linux-sctp, netdev, Vlad Yasevich, Neil Horman, David S . Miller,
	David Laight

On Wed, May 20, 2020 at 06:15:31PM +0300, Jere Leppänen wrote:
> Commit bdf6fa52f01b ("sctp: handle association restarts when the
> socket is closed.") starts shutdown when an association is restarted,
> if in SHUTDOWN-PENDING state and the socket is closed. However, the
> rationale stated in that commit applies also when in SHUTDOWN-SENT
> state - we don't want to move an association to ESTABLISHED state when
> the socket has been closed, because that results in an association
> that is unreachable from user space.
> 
> The problem scenario:
> 
> 1.  Client crashes and/or restarts.
> 
> 2.  Server (using one-to-one socket) calls close(). SHUTDOWN is lost.
> 
> 3.  Client reconnects using the same addresses and ports.
> 
> 4.  Server's association is restarted. The association and the socket
>     move to ESTABLISHED state, even though the server process has
>     closed its descriptor.
> 
> Also, after step 4 when the server process exits, some resources are
> leaked in an attempt to release the underlying inet sock structure in
> ESTABLISHED state:
> 
>     IPv4: Attempt to release TCP socket in state 1 00000000377288c7
> 
> Fix by acting the same way as in SHUTDOWN-PENDING state. That is, if
> an association is restarted in SHUTDOWN-SENT state and the socket is
> closed, then start shutdown and don't move the association or the
> socket to ESTABLISHED state.
> 
> Fixes: bdf6fa52f01b ("sctp: handle association restarts when the socket is closed.")
> Signed-off-by: Jere Leppänen <jere.leppanen@nokia.com>
> ---
>  net/sctp/sm_statefuns.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index 26788f4a3b9e..e86620fbd90f 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -1856,12 +1856,13 @@ static enum sctp_disposition sctp_sf_do_dupcook_a(
>  	/* Update the content of current association. */
>  	sctp_add_cmd_sf(commands, SCTP_CMD_UPDATE_ASSOC, SCTP_ASOC(new_asoc));
>  	sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP, SCTP_ULPEVENT(ev));
> -	if (sctp_state(asoc, SHUTDOWN_PENDING) &&
> +	if ((sctp_state(asoc, SHUTDOWN_PENDING) ||
> +	     sctp_state(asoc, SHUTDOWN_SENT)) &&
>  	    (sctp_sstate(asoc->base.sk, CLOSING) ||
>  	     sock_flag(asoc->base.sk, SOCK_DEAD))) {
> -		/* if were currently in SHUTDOWN_PENDING, but the socket
> -		 * has been closed by user, don't transition to ESTABLISHED.
> -		 * Instead trigger SHUTDOWN bundled with COOKIE_ACK.
> +		/* If the socket has been closed by user, don't
> +		 * transition to ESTABLISHED. Instead trigger SHUTDOWN
> +		 * bundled with COOKIE_ACK.
>  		 */
>  		sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(repl));
>  		return sctp_sf_do_9_2_start_shutdown(net, ep, asoc,
> 
> base-commit: 20a785aa52c82246055a089e55df9dac47d67da1

This last line is not standard, but git didn't complain about it here.

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

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

* Re: [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket
@ 2020-05-20 19:46   ` Marcelo Ricardo Leitner
  0 siblings, 0 replies; 8+ messages in thread
From: Marcelo Ricardo Leitner @ 2020-05-20 19:46 UTC (permalink / raw)
  To: Jere Leppänen
  Cc: linux-sctp, netdev, Vlad Yasevich, Neil Horman, David S . Miller,
	David Laight

On Wed, May 20, 2020 at 06:15:31PM +0300, Jere Leppänen wrote:
> Commit bdf6fa52f01b ("sctp: handle association restarts when the
> socket is closed.") starts shutdown when an association is restarted,
> if in SHUTDOWN-PENDING state and the socket is closed. However, the
> rationale stated in that commit applies also when in SHUTDOWN-SENT
> state - we don't want to move an association to ESTABLISHED state when
> the socket has been closed, because that results in an association
> that is unreachable from user space.
> 
> The problem scenario:
> 
> 1.  Client crashes and/or restarts.
> 
> 2.  Server (using one-to-one socket) calls close(). SHUTDOWN is lost.
> 
> 3.  Client reconnects using the same addresses and ports.
> 
> 4.  Server's association is restarted. The association and the socket
>     move to ESTABLISHED state, even though the server process has
>     closed its descriptor.
> 
> Also, after step 4 when the server process exits, some resources are
> leaked in an attempt to release the underlying inet sock structure in
> ESTABLISHED state:
> 
>     IPv4: Attempt to release TCP socket in state 1 00000000377288c7
> 
> Fix by acting the same way as in SHUTDOWN-PENDING state. That is, if
> an association is restarted in SHUTDOWN-SENT state and the socket is
> closed, then start shutdown and don't move the association or the
> socket to ESTABLISHED state.
> 
> Fixes: bdf6fa52f01b ("sctp: handle association restarts when the socket is closed.")
> Signed-off-by: Jere Leppänen <jere.leppanen@nokia.com>
> ---
>  net/sctp/sm_statefuns.c | 9 +++++----
>  1 file changed, 5 insertions(+), 4 deletions(-)
> 
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index 26788f4a3b9e..e86620fbd90f 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -1856,12 +1856,13 @@ static enum sctp_disposition sctp_sf_do_dupcook_a(
>  	/* Update the content of current association. */
>  	sctp_add_cmd_sf(commands, SCTP_CMD_UPDATE_ASSOC, SCTP_ASOC(new_asoc));
>  	sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP, SCTP_ULPEVENT(ev));
> -	if (sctp_state(asoc, SHUTDOWN_PENDING) &&
> +	if ((sctp_state(asoc, SHUTDOWN_PENDING) ||
> +	     sctp_state(asoc, SHUTDOWN_SENT)) &&
>  	    (sctp_sstate(asoc->base.sk, CLOSING) ||
>  	     sock_flag(asoc->base.sk, SOCK_DEAD))) {
> -		/* if were currently in SHUTDOWN_PENDING, but the socket
> -		 * has been closed by user, don't transition to ESTABLISHED.
> -		 * Instead trigger SHUTDOWN bundled with COOKIE_ACK.
> +		/* If the socket has been closed by user, don't
> +		 * transition to ESTABLISHED. Instead trigger SHUTDOWN
> +		 * bundled with COOKIE_ACK.
>  		 */
>  		sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(repl));
>  		return sctp_sf_do_9_2_start_shutdown(net, ep, asoc,
> 
> base-commit: 20a785aa52c82246055a089e55df9dac47d67da1

This last line is not standard, but git didn't complain about it here.

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

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

* Re: [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket is closed
  2020-05-20 19:46   ` [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket Marcelo Ricardo Leitner
@ 2020-05-20 21:50     ` Jere Leppänen
  -1 siblings, 0 replies; 8+ messages in thread
From: Jere Leppänen @ 2020-05-20 21:50 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Jere Leppänen, linux-sctp, netdev, Vlad Yasevich,
	Neil Horman, David S . Miller, David Laight

[-- Attachment #1: Type: text/plain, Size: 3360 bytes --]

On Wed, 20 May 2020, Marcelo Ricardo Leitner wrote:

> On Wed, May 20, 2020 at 06:15:31PM +0300, Jere Leppänen wrote:
> > Commit bdf6fa52f01b ("sctp: handle association restarts when the
> > socket is closed.") starts shutdown when an association is restarted,
> > if in SHUTDOWN-PENDING state and the socket is closed. However, the
> > rationale stated in that commit applies also when in SHUTDOWN-SENT
> > state - we don't want to move an association to ESTABLISHED state when
> > the socket has been closed, because that results in an association
> > that is unreachable from user space.
> > 
> > The problem scenario:
> > 
> > 1.  Client crashes and/or restarts.
> > 
> > 2.  Server (using one-to-one socket) calls close(). SHUTDOWN is lost.
> > 
> > 3.  Client reconnects using the same addresses and ports.
> > 
> > 4.  Server's association is restarted. The association and the socket
> >     move to ESTABLISHED state, even though the server process has
> >     closed its descriptor.
> > 
> > Also, after step 4 when the server process exits, some resources are
> > leaked in an attempt to release the underlying inet sock structure in
> > ESTABLISHED state:
> > 
> >     IPv4: Attempt to release TCP socket in state 1 00000000377288c7
> > 
> > Fix by acting the same way as in SHUTDOWN-PENDING state. That is, if
> > an association is restarted in SHUTDOWN-SENT state and the socket is
> > closed, then start shutdown and don't move the association or the
> > socket to ESTABLISHED state.
> > 
> > Fixes: bdf6fa52f01b ("sctp: handle association restarts when the socket is closed.")
> > Signed-off-by: Jere Leppänen <jere.leppanen@nokia.com>
> > ---
> >  net/sctp/sm_statefuns.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> > index 26788f4a3b9e..e86620fbd90f 100644
> > --- a/net/sctp/sm_statefuns.c
> > +++ b/net/sctp/sm_statefuns.c
> > @@ -1856,12 +1856,13 @@ static enum sctp_disposition sctp_sf_do_dupcook_a(
> >  	/* Update the content of current association. */
> >  	sctp_add_cmd_sf(commands, SCTP_CMD_UPDATE_ASSOC, SCTP_ASOC(new_asoc));
> >  	sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP, SCTP_ULPEVENT(ev));
> > -	if (sctp_state(asoc, SHUTDOWN_PENDING) &&
> > +	if ((sctp_state(asoc, SHUTDOWN_PENDING) ||
> > +	     sctp_state(asoc, SHUTDOWN_SENT)) &&
> >  	    (sctp_sstate(asoc->base.sk, CLOSING) ||
> >  	     sock_flag(asoc->base.sk, SOCK_DEAD))) {
> > -		/* if were currently in SHUTDOWN_PENDING, but the socket
> > -		 * has been closed by user, don't transition to ESTABLISHED.
> > -		 * Instead trigger SHUTDOWN bundled with COOKIE_ACK.
> > +		/* If the socket has been closed by user, don't
> > +		 * transition to ESTABLISHED. Instead trigger SHUTDOWN
> > +		 * bundled with COOKIE_ACK.
> >  		 */
> >  		sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(repl));
> >  		return sctp_sf_do_9_2_start_shutdown(net, ep, asoc,
> > 
> > base-commit: 20a785aa52c82246055a089e55df9dac47d67da1
> 
> This last line is not standard, but git didn't complain about it here.

The git format-patch --base option. It's mentioned in the docs:

https://www.kernel.org/doc/html/v5.6/process/submitting-patches.html#providing-base-tree-information

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

Thanks for the speedy ack, Marcelo.

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

* Re: [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket
@ 2020-05-20 21:50     ` Jere Leppänen
  0 siblings, 0 replies; 8+ messages in thread
From: Jere Leppänen @ 2020-05-20 21:50 UTC (permalink / raw)
  To: Marcelo Ricardo Leitner
  Cc: Jere Leppänen, linux-sctp, netdev, Vlad Yasevich,
	Neil Horman, David S . Miller, David Laight

[-- Attachment #1: Type: text/plain, Size: 3434 bytes --]

On Wed, 20 May 2020, Marcelo Ricardo Leitner wrote:

> On Wed, May 20, 2020 at 06:15:31PM +0300, Jere Leppänen wrote:
> > Commit bdf6fa52f01b ("sctp: handle association restarts when the
> > socket is closed.") starts shutdown when an association is restarted,
> > if in SHUTDOWN-PENDING state and the socket is closed. However, the
> > rationale stated in that commit applies also when in SHUTDOWN-SENT
> > state - we don't want to move an association to ESTABLISHED state when
> > the socket has been closed, because that results in an association
> > that is unreachable from user space.
> > 
> > The problem scenario:
> > 
> > 1.  Client crashes and/or restarts.
> > 
> > 2.  Server (using one-to-one socket) calls close(). SHUTDOWN is lost.
> > 
> > 3.  Client reconnects using the same addresses and ports.
> > 
> > 4.  Server's association is restarted. The association and the socket
> >     move to ESTABLISHED state, even though the server process has
> >     closed its descriptor.
> > 
> > Also, after step 4 when the server process exits, some resources are
> > leaked in an attempt to release the underlying inet sock structure in
> > ESTABLISHED state:
> > 
> >     IPv4: Attempt to release TCP socket in state 1 00000000377288c7
> > 
> > Fix by acting the same way as in SHUTDOWN-PENDING state. That is, if
> > an association is restarted in SHUTDOWN-SENT state and the socket is
> > closed, then start shutdown and don't move the association or the
> > socket to ESTABLISHED state.
> > 
> > Fixes: bdf6fa52f01b ("sctp: handle association restarts when the socket is closed.")
> > Signed-off-by: Jere Leppänen <jere.leppanen@nokia.com>
> > ---
> >  net/sctp/sm_statefuns.c | 9 +++++----
> >  1 file changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> > index 26788f4a3b9e..e86620fbd90f 100644
> > --- a/net/sctp/sm_statefuns.c
> > +++ b/net/sctp/sm_statefuns.c
> > @@ -1856,12 +1856,13 @@ static enum sctp_disposition sctp_sf_do_dupcook_a(
> >  	/* Update the content of current association. */
> >  	sctp_add_cmd_sf(commands, SCTP_CMD_UPDATE_ASSOC, SCTP_ASOC(new_asoc));
> >  	sctp_add_cmd_sf(commands, SCTP_CMD_EVENT_ULP, SCTP_ULPEVENT(ev));
> > -	if (sctp_state(asoc, SHUTDOWN_PENDING) &&
> > +	if ((sctp_state(asoc, SHUTDOWN_PENDING) ||
> > +	     sctp_state(asoc, SHUTDOWN_SENT)) &&
> >  	    (sctp_sstate(asoc->base.sk, CLOSING) ||
> >  	     sock_flag(asoc->base.sk, SOCK_DEAD))) {
> > -		/* if were currently in SHUTDOWN_PENDING, but the socket
> > -		 * has been closed by user, don't transition to ESTABLISHED.
> > -		 * Instead trigger SHUTDOWN bundled with COOKIE_ACK.
> > +		/* If the socket has been closed by user, don't
> > +		 * transition to ESTABLISHED. Instead trigger SHUTDOWN
> > +		 * bundled with COOKIE_ACK.
> >  		 */
> >  		sctp_add_cmd_sf(commands, SCTP_CMD_REPLY, SCTP_CHUNK(repl));
> >  		return sctp_sf_do_9_2_start_shutdown(net, ep, asoc,
> > 
> > base-commit: 20a785aa52c82246055a089e55df9dac47d67da1
> 
> This last line is not standard, but git didn't complain about it here.

The git format-patch --base option. It's mentioned in the docs:

https://www.kernel.org/doc/html/v5.6/process/submitting-patches.html#providing-base-tree-information

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

Thanks for the speedy ack, Marcelo.

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

* Re: [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket is closed
  2020-05-20 15:15 ` [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket is Jere Leppänen
@ 2020-05-22 22:48   ` David Miller
  -1 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2020-05-22 22:48 UTC (permalink / raw)
  To: jere.leppanen
  Cc: linux-sctp, netdev, vyasevich, nhorman, marcelo.leitner, David.Laight

From: Jere Leppänen <jere.leppanen@nokia.com>
Date: Wed, 20 May 2020 18:15:31 +0300

> Commit bdf6fa52f01b ("sctp: handle association restarts when the
> socket is closed.") starts shutdown when an association is restarted,
> if in SHUTDOWN-PENDING state and the socket is closed. However, the
> rationale stated in that commit applies also when in SHUTDOWN-SENT
> state - we don't want to move an association to ESTABLISHED state when
> the socket has been closed, because that results in an association
> that is unreachable from user space.
> 
> The problem scenario:
> 
> 1.  Client crashes and/or restarts.
> 
> 2.  Server (using one-to-one socket) calls close(). SHUTDOWN is lost.
> 
> 3.  Client reconnects using the same addresses and ports.
> 
> 4.  Server's association is restarted. The association and the socket
>     move to ESTABLISHED state, even though the server process has
>     closed its descriptor.
> 
> Also, after step 4 when the server process exits, some resources are
> leaked in an attempt to release the underlying inet sock structure in
> ESTABLISHED state:
> 
>     IPv4: Attempt to release TCP socket in state 1 00000000377288c7
> 
> Fix by acting the same way as in SHUTDOWN-PENDING state. That is, if
> an association is restarted in SHUTDOWN-SENT state and the socket is
> closed, then start shutdown and don't move the association or the
> socket to ESTABLISHED state.
> 
> Fixes: bdf6fa52f01b ("sctp: handle association restarts when the socket is closed.")
> Signed-off-by: Jere Leppänen <jere.leppanen@nokia.com>

Applied and queued up for -stable, thanks.

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

* Re: [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket
@ 2020-05-22 22:48   ` David Miller
  0 siblings, 0 replies; 8+ messages in thread
From: David Miller @ 2020-05-22 22:48 UTC (permalink / raw)
  To: jere.leppanen
  Cc: linux-sctp, netdev, vyasevich, nhorman, marcelo.leitner, David.Laight

From: Jere Leppänen <jere.leppanen@nokia.com>
Date: Wed, 20 May 2020 18:15:31 +0300

> Commit bdf6fa52f01b ("sctp: handle association restarts when the
> socket is closed.") starts shutdown when an association is restarted,
> if in SHUTDOWN-PENDING state and the socket is closed. However, the
> rationale stated in that commit applies also when in SHUTDOWN-SENT
> state - we don't want to move an association to ESTABLISHED state when
> the socket has been closed, because that results in an association
> that is unreachable from user space.
> 
> The problem scenario:
> 
> 1.  Client crashes and/or restarts.
> 
> 2.  Server (using one-to-one socket) calls close(). SHUTDOWN is lost.
> 
> 3.  Client reconnects using the same addresses and ports.
> 
> 4.  Server's association is restarted. The association and the socket
>     move to ESTABLISHED state, even though the server process has
>     closed its descriptor.
> 
> Also, after step 4 when the server process exits, some resources are
> leaked in an attempt to release the underlying inet sock structure in
> ESTABLISHED state:
> 
>     IPv4: Attempt to release TCP socket in state 1 00000000377288c7
> 
> Fix by acting the same way as in SHUTDOWN-PENDING state. That is, if
> an association is restarted in SHUTDOWN-SENT state and the socket is
> closed, then start shutdown and don't move the association or the
> socket to ESTABLISHED state.
> 
> Fixes: bdf6fa52f01b ("sctp: handle association restarts when the socket is closed.")
> Signed-off-by: Jere Leppänen <jere.leppanen@nokia.com>

Applied and queued up for -stable, thanks.

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

end of thread, other threads:[~2020-05-22 22:48 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-20 15:15 [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket is closed Jere Leppänen
2020-05-20 15:15 ` [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket is Jere Leppänen
2020-05-20 19:46 ` [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket is closed Marcelo Ricardo Leitner
2020-05-20 19:46   ` [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket Marcelo Ricardo Leitner
2020-05-20 21:50   ` [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket is closed Jere Leppänen
2020-05-20 21:50     ` [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket Jere Leppänen
2020-05-22 22:48 ` [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket is closed David Miller
2020-05-22 22:48   ` [PATCH net 1/1] sctp: Start shutdown on association restart if in SHUTDOWN-SENT state and socket 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.