* [PATCH v2 1/2] net: sctp: socket: Fix uninitialized error on socket shutdown
@ 2026-07-16 10:04 luoqing
2026-07-16 10:04 ` [PATCH v2 2/2] net: sctp: auth: Fix safety issue when skb_clone fails in auth_chunk handling luoqing
2026-07-17 15:02 ` [PATCH v2 1/2] net: sctp: socket: Fix uninitialized error on socket shutdown Xin Long
0 siblings, 2 replies; 5+ messages in thread
From: luoqing @ 2026-07-16 10:04 UTC (permalink / raw)
To: marcelo.leitner, lucien.xin, davem, edumazet, kuba, pabeni
Cc: horms, linux-sctp, netdev, linux-kernel, luoqing
From: luoqing <luoqing@kylinos.cn>
When sctp_skb_recv_datagram() detects sk->sk_shutdown & RCV_SHUTDOWN,
it breaks out of the loop and returns NULL without setting *err.
This leaves the error pointer uninitialized or with a stale value,
which can confuse callers expecting a clean shutdown indication.
Compare with the generic __skb_wait_for_more_packets() in
net/core/datagram.c which properly handles shutdown by setting *err = 0.
Fix this by setting *err = 0 before breaking when the socket is shut down,
indicating an orderly shutdown rather than an error condition.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: luoqing <luoqing@kylinos.cn>
---
net/sctp/socket.c | 4 +++-
1 file changed, 3 insertions(+), 1 deletion(-)
diff --git a/net/sctp/socket.c b/net/sctp/socket.c
index c7b9e325ec1c..ea7050b27715 100644
--- a/net/sctp/socket.c
+++ b/net/sctp/socket.c
@@ -9117,8 +9117,10 @@ struct sk_buff *sctp_skb_recv_datagram(struct sock *sk, int flags, int *err)
if (error)
goto no_packet;
- if (sk->sk_shutdown & RCV_SHUTDOWN)
+ if (sk->sk_shutdown & RCV_SHUTDOWN) {
+ *err = 0;
break;
+ }
/* User doesn't want to wait. */
--
2.25.1
^ permalink raw reply related [flat|nested] 5+ messages in thread* [PATCH v2 2/2] net: sctp: auth: Fix safety issue when skb_clone fails in auth_chunk handling 2026-07-16 10:04 [PATCH v2 1/2] net: sctp: socket: Fix uninitialized error on socket shutdown luoqing @ 2026-07-16 10:04 ` luoqing 2026-07-17 15:01 ` Xin Long 2026-07-17 15:02 ` [PATCH v2 1/2] net: sctp: socket: Fix uninitialized error on socket shutdown Xin Long 1 sibling, 1 reply; 5+ messages in thread From: luoqing @ 2026-07-16 10:04 UTC (permalink / raw) To: marcelo.leitner, lucien.xin, davem, edumazet, kuba, pabeni Cc: horms, linux-sctp, netdev, linux-kernel, luoqing From: luoqing <luoqing@kylinos.cn> When processing AUTH + COOKIE-ECHO packets, if skb_clone fails due to memory pressure, chunk->auth_chunk is set to NULL but chunk->auth is still set to 1. This causes sctp_auth_chunk_verify to skip the AUTH validation (since auth_chunk is NULL), allowing unauthenticated COOKIE-ECHO packets to be accepted. Fix this by only setting chunk->auth = 1 when skb_clone succeeds. Fixes: 59d8d4434f429b ("sctp: delay the authentication for the duplicated cookie-echo chunk") Signed-off-by: luoqing <luoqing@kylinos.cn> --- net/sctp/associola.c | 3 ++- net/sctp/endpointola.c | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/net/sctp/associola.c b/net/sctp/associola.c index 62d3cc155809..e54068305396 100644 --- a/net/sctp/associola.c +++ b/net/sctp/associola.c @@ -999,7 +999,8 @@ static void sctp_assoc_bh_rcv(struct work_struct *work) if (next_hdr->type == SCTP_CID_COOKIE_ECHO) { chunk->auth_chunk = skb_clone(chunk->skb, GFP_ATOMIC); - chunk->auth = 1; + if (chunk->auth_chunk) + chunk->auth = 1; continue; } } diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c index dfb1719275db..3419748c66bc 100644 --- a/net/sctp/endpointola.c +++ b/net/sctp/endpointola.c @@ -368,7 +368,8 @@ static void sctp_endpoint_bh_rcv(struct work_struct *work) if (next_hdr->type == SCTP_CID_COOKIE_ECHO) { chunk->auth_chunk = skb_clone(chunk->skb, GFP_ATOMIC); - chunk->auth = 1; + if (chunk->auth_chunk) + chunk->auth = 1; continue; } } -- 2.25.1 ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] net: sctp: auth: Fix safety issue when skb_clone fails in auth_chunk handling 2026-07-16 10:04 ` [PATCH v2 2/2] net: sctp: auth: Fix safety issue when skb_clone fails in auth_chunk handling luoqing @ 2026-07-17 15:01 ` Xin Long 2026-07-17 15:22 ` Xin Long 0 siblings, 1 reply; 5+ messages in thread From: Xin Long @ 2026-07-17 15:01 UTC (permalink / raw) To: luoqing Cc: marcelo.leitner, davem, edumazet, kuba, pabeni, horms, linux-sctp, netdev, linux-kernel, luoqing On Thu, Jul 16, 2026 at 6:05 AM luoqing <l1138897701@163.com> wrote: > > From: luoqing <luoqing@kylinos.cn> > > When processing AUTH + COOKIE-ECHO packets, if skb_clone fails due to > memory pressure, chunk->auth_chunk is set to NULL but chunk->auth is > still set to 1. This causes sctp_auth_chunk_verify to skip the AUTH > validation (since auth_chunk is NULL), allowing unauthenticated > COOKIE-ECHO packets to be accepted. > > Fix this by only setting chunk->auth = 1 when skb_clone succeeds. > > Fixes: 59d8d4434f429b ("sctp: delay the authentication for the duplicated cookie-echo chunk") > > Signed-off-by: luoqing <luoqing@kylinos.cn> > --- > net/sctp/associola.c | 3 ++- > net/sctp/endpointola.c | 3 ++- > 2 files changed, 4 insertions(+), 2 deletions(-) > > diff --git a/net/sctp/associola.c b/net/sctp/associola.c > index 62d3cc155809..e54068305396 100644 > --- a/net/sctp/associola.c > +++ b/net/sctp/associola.c > @@ -999,7 +999,8 @@ static void sctp_assoc_bh_rcv(struct work_struct *work) > if (next_hdr->type == SCTP_CID_COOKIE_ECHO) { > chunk->auth_chunk = skb_clone(chunk->skb, > GFP_ATOMIC); > - chunk->auth = 1; > + if (chunk->auth_chunk) > + chunk->auth = 1; > continue; > } > } > diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c > index dfb1719275db..3419748c66bc 100644 > --- a/net/sctp/endpointola.c > +++ b/net/sctp/endpointola.c > @@ -368,7 +368,8 @@ static void sctp_endpoint_bh_rcv(struct work_struct *work) > if (next_hdr->type == SCTP_CID_COOKIE_ECHO) { > chunk->auth_chunk = skb_clone(chunk->skb, > GFP_ATOMIC); > - chunk->auth = 1; > + if (chunk->auth_chunk) > + chunk->auth = 1; > continue; > } > } > -- > 2.25.1 > The patch does not fully address the issues reported by Sashiko in the following two cases (I will interpret them below): 1. [sashiko-gemini]: When skb_clone() fails, chunk->auth_chunk is set to NULL and chunk->auth remains 0. In this case, the chunk will be dropped/skipped in sctp_assoc_bh_rcv() by the following check: if (sctp_auth_recv_cid(subtype.chunk, asoc) && !chunk->auth) continue; This works as expected because the chunk is skipped. However, in sctp_endpoint_bh_rcv(), asoc is NULL for new connections, so the above condition is always false. As a result, the COOKIE-ECHO chunk can still be processed with chunk->auth_chunk == NULL. 2. [sashiko-claude]: When the association requires authentication for the COOKIE-ECHO chunk, but the packet does not contain an AUTH chunk, skb_clone() is never called and chunk->auth_chunk remains NULL. Since the new association is also NULL in sctp_endpoint_bh_rcv(), the check above cannot catch this case either. The COOKIE-ECHO chunk will still be processed with chunk->auth_chunk == NULL. A better fix would be: Add a check in sctp_auth_chunk_verify() at the point where the COOKIE-ECHO chunk is actually being processed: if (!chunk->auth_chunk) return !sctp_auth_recv_cid(chunk->chunk_hdr->type, asoc); This ensures that if chunk->auth_chunk is missing while authentication is required for the COOKIE-ECHO chunk, the verification fails and the chunk is dropped. Otherwise, when authentication is not required, processing can continue normally. Please give it a try. Thanks. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 2/2] net: sctp: auth: Fix safety issue when skb_clone fails in auth_chunk handling 2026-07-17 15:01 ` Xin Long @ 2026-07-17 15:22 ` Xin Long 0 siblings, 0 replies; 5+ messages in thread From: Xin Long @ 2026-07-17 15:22 UTC (permalink / raw) To: luoqing Cc: marcelo.leitner, davem, edumazet, kuba, pabeni, horms, linux-sctp, netdev, linux-kernel, luoqing On Fri, Jul 17, 2026 at 11:01 AM Xin Long <lucien.xin@gmail.com> wrote: > > On Thu, Jul 16, 2026 at 6:05 AM luoqing <l1138897701@163.com> wrote: > > > > From: luoqing <luoqing@kylinos.cn> > > > > When processing AUTH + COOKIE-ECHO packets, if skb_clone fails due to > > memory pressure, chunk->auth_chunk is set to NULL but chunk->auth is > > still set to 1. This causes sctp_auth_chunk_verify to skip the AUTH > > validation (since auth_chunk is NULL), allowing unauthenticated > > COOKIE-ECHO packets to be accepted. > > > > Fix this by only setting chunk->auth = 1 when skb_clone succeeds. > > > > Fixes: 59d8d4434f429b ("sctp: delay the authentication for the duplicated cookie-echo chunk") > > > > Signed-off-by: luoqing <luoqing@kylinos.cn> > > --- > > net/sctp/associola.c | 3 ++- > > net/sctp/endpointola.c | 3 ++- > > 2 files changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/net/sctp/associola.c b/net/sctp/associola.c > > index 62d3cc155809..e54068305396 100644 > > --- a/net/sctp/associola.c > > +++ b/net/sctp/associola.c > > @@ -999,7 +999,8 @@ static void sctp_assoc_bh_rcv(struct work_struct *work) > > if (next_hdr->type == SCTP_CID_COOKIE_ECHO) { > > chunk->auth_chunk = skb_clone(chunk->skb, > > GFP_ATOMIC); > > - chunk->auth = 1; > > + if (chunk->auth_chunk) > > + chunk->auth = 1; > > continue; > > } > > } > > diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c > > index dfb1719275db..3419748c66bc 100644 > > --- a/net/sctp/endpointola.c > > +++ b/net/sctp/endpointola.c > > @@ -368,7 +368,8 @@ static void sctp_endpoint_bh_rcv(struct work_struct *work) > > if (next_hdr->type == SCTP_CID_COOKIE_ECHO) { > > chunk->auth_chunk = skb_clone(chunk->skb, > > GFP_ATOMIC); > > - chunk->auth = 1; > > + if (chunk->auth_chunk) > > + chunk->auth = 1; > > continue; > > } > > } > > -- > > 2.25.1 > > > The patch does not fully address the issues reported by Sashiko in the > following two cases (I will interpret them below): > > 1. [sashiko-gemini]: > > When skb_clone() fails, chunk->auth_chunk is set to NULL and chunk->auth > remains 0. In this case, the chunk will be dropped/skipped in > sctp_assoc_bh_rcv() by the following check: > > if (sctp_auth_recv_cid(subtype.chunk, asoc) && !chunk->auth) > continue; > > This works as expected because the chunk is skipped. However, in > sctp_endpoint_bh_rcv(), asoc is NULL for new connections, so the above > condition is always false. As a result, the COOKIE-ECHO chunk can still be > processed with chunk->auth_chunk == NULL. > > 2. [sashiko-claude]: > > When the association requires authentication for the COOKIE-ECHO chunk, but > the packet does not contain an AUTH chunk, skb_clone() is never called and > chunk->auth_chunk remains NULL. Since the new association is also NULL in > sctp_endpoint_bh_rcv(), the check above cannot catch this case either. The > COOKIE-ECHO chunk will still be processed with chunk->auth_chunk == NULL. > > A better fix would be: > > Add a check in sctp_auth_chunk_verify() at the point where the COOKIE-ECHO > chunk is actually being processed: > > > if (!chunk->auth_chunk) > return !sctp_auth_recv_cid(chunk->chunk_hdr->type, asoc); > > This ensures that if chunk->auth_chunk is missing while authentication is > required for the COOKIE-ECHO chunk, the verification fails and the chunk is > dropped. Otherwise, when authentication is not required, processing can > continue normally. > > Please give it a try. > Also, please add a extra Fixes tag in your next post: Fixes: bbd0d59809f9 ("[SCTP]: Implement the receive and verification of AUTH chunk") which introduces chunk->auth_chunk and calls skb_clone() in sctp_endpoint_bh_rcv(). Thanks. ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v2 1/2] net: sctp: socket: Fix uninitialized error on socket shutdown 2026-07-16 10:04 [PATCH v2 1/2] net: sctp: socket: Fix uninitialized error on socket shutdown luoqing 2026-07-16 10:04 ` [PATCH v2 2/2] net: sctp: auth: Fix safety issue when skb_clone fails in auth_chunk handling luoqing @ 2026-07-17 15:02 ` Xin Long 1 sibling, 0 replies; 5+ messages in thread From: Xin Long @ 2026-07-17 15:02 UTC (permalink / raw) To: luoqing Cc: marcelo.leitner, davem, edumazet, kuba, pabeni, horms, linux-sctp, netdev, linux-kernel, luoqing On Thu, Jul 16, 2026 at 6:05 AM luoqing <l1138897701@163.com> wrote: > > From: luoqing <luoqing@kylinos.cn> > > When sctp_skb_recv_datagram() detects sk->sk_shutdown & RCV_SHUTDOWN, > it breaks out of the loop and returns NULL without setting *err. > This leaves the error pointer uninitialized or with a stale value, > which can confuse callers expecting a clean shutdown indication. > > Compare with the generic __skb_wait_for_more_packets() in > net/core/datagram.c which properly handles shutdown by setting *err = 0. > > Fix this by setting *err = 0 before breaking when the socket is shut down, > indicating an orderly shutdown rather than an error condition. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > > Signed-off-by: luoqing <luoqing@kylinos.cn> > --- > net/sctp/socket.c | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) > > diff --git a/net/sctp/socket.c b/net/sctp/socket.c > index c7b9e325ec1c..ea7050b27715 100644 > --- a/net/sctp/socket.c > +++ b/net/sctp/socket.c > @@ -9117,8 +9117,10 @@ struct sk_buff *sctp_skb_recv_datagram(struct sock *sk, int flags, int *err) > if (error) > goto no_packet; > > - if (sk->sk_shutdown & RCV_SHUTDOWN) > + if (sk->sk_shutdown & RCV_SHUTDOWN) { > + *err = 0; > break; > + } > > > /* User doesn't want to wait. */ > -- > 2.25.1 > This patch doesn't really fix issues, please drop it. Thanks. ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2026-07-17 15:22 UTC | newest] Thread overview: 5+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-16 10:04 [PATCH v2 1/2] net: sctp: socket: Fix uninitialized error on socket shutdown luoqing 2026-07-16 10:04 ` [PATCH v2 2/2] net: sctp: auth: Fix safety issue when skb_clone fails in auth_chunk handling luoqing 2026-07-17 15:01 ` Xin Long 2026-07-17 15:22 ` Xin Long 2026-07-17 15:02 ` [PATCH v2 1/2] net: sctp: socket: Fix uninitialized error on socket shutdown Xin Long
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox