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