sctp: auth: break when skb_clone fails for auth_chunk

[PATCH v4 net-next] sctp: auth: break when skb_clone fails for auth_chunk

Posted by luoqing 3 days, 23 hours ago

From: Qing Luo <luoqing@kylinos.cn>

When processing AUTH + COOKIE-ECHO packets, if skb_clone() fails
due to memory pressure, chunk->auth_chunk is NULL. The original
code still sets chunk->auth = 1 and continues, leaving the
COOKIE-ECHO to be processed without a valid auth_chunk for
deferred verification.

The intent of not setting auth was to drop the chunk earlier,
but in sctp_endpoint_bh_rcv() asoc is NULL for new connections,
so sctp_auth_recv_cid() returns 0 and the early check is
ineffective.

Fix by breaking out of the receive loop when skb_clone() fails,
dropping the entire packet since the AUTH data needed for
COOKIE-ECHO verification cannot be preserved.

Fixes: bbd0d59809f9 ("[SCTP]: Implement the receive and verification of AUTH chunk")
Signed-off-by: Qing Luo <luoqing@kylinos.cn>
---
 net/sctp/associola.c   | 2 ++
 net/sctp/endpointola.c | 2 ++
 2 files changed, 4 insertions(+)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 62d3cc155809..7741f982e368 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -999,6 +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);
+				if (!chunk->auth_chunk)
+					break;
 				chunk->auth = 1;
 				continue;
 			}
diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index dfb1719275db..9675370a46da 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -368,6 +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);
+				if (!chunk->auth_chunk)
+					break;
 				chunk->auth = 1;
 				continue;
 			}
-- 
2.25.1

Re: [PATCH v4 net-next] sctp: auth: break when skb_clone fails for auth_chunk

Posted by Xin Long 2 days, 6 hours ago

On Mon, Jul 20, 2026 at 9:56 PM luoqing <l1138897701@163.com> wrote:
>
> From: Qing Luo <luoqing@kylinos.cn>
>
> When processing AUTH + COOKIE-ECHO packets, if skb_clone() fails
> due to memory pressure, chunk->auth_chunk is NULL. The original
> code still sets chunk->auth = 1 and continues, leaving the
> COOKIE-ECHO to be processed without a valid auth_chunk for
> deferred verification.
>
> The intent of not setting auth was to drop the chunk earlier,
> but in sctp_endpoint_bh_rcv() asoc is NULL for new connections,
> so sctp_auth_recv_cid() returns 0 and the early check is
> ineffective.
>
> Fix by breaking out of the receive loop when skb_clone() fails,
> dropping the entire packet since the AUTH data needed for
> COOKIE-ECHO verification cannot be preserved.
>
> Fixes: bbd0d59809f9 ("[SCTP]: Implement the receive and verification of AUTH chunk")
> Signed-off-by: Qing Luo <luoqing@kylinos.cn>
> ---
>  net/sctp/associola.c   | 2 ++
>  net/sctp/endpointola.c | 2 ++
>  2 files changed, 4 insertions(+)
>
> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
> index 62d3cc155809..7741f982e368 100644
> --- a/net/sctp/associola.c
> +++ b/net/sctp/associola.c
> @@ -999,6 +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);
> +                               if (!chunk->auth_chunk)
> +                                       break;
>                                 chunk->auth = 1;
>                                 continue;
>                         }
> diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
> index dfb1719275db..9675370a46da 100644
> --- a/net/sctp/endpointola.c
> +++ b/net/sctp/endpointola.c
> @@ -368,6 +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);
> +                               if (!chunk->auth_chunk)
> +                                       break;
>                                 chunk->auth = 1;
>                                 continue;
>                         }
> --
> 2.25.1
>
>

The sashiko suggests adding chunk->pdiscard = 1 and continue to avoid
stalling the queue.

                                if (!chunk->auth_chunk) {
                                        chunk->pdiscard = 1;
                                        continue;
                                }

Also, as the original issue was already addressed in the other patch,
you should delete the "Fixes:" tag from this patch. This patch is more
like to discard the packet early if the skb_clone() fails.

Thanks.

[PATCH net-next v5] sctp: auth: discard auth_chunk when skb_clone fails

Posted by luoqing 1 day, 18 hours ago

From: Qing Luo <luoqing@kylinos.cn>

When processing AUTH + COOKIE-ECHO packets, if skb_clone() fails
due to memory pressure, chunk->auth_chunk is NULL. The original
code still sets chunk->auth = 1 and continues, leaving the
COOKIE-ECHO to be processed without a valid auth_chunk for
deferred verification.

Discard the AUTH chunk early via pdiscard when skb_clone() fails,
so that the receive loop can continue processing remaining chunks
in the inqueue instead of stalling the entire packet.

Signed-off-by: Qing Luo <luoqing@kylinos.cn>
---
 net/sctp/associola.c   | 4 ++++
 net/sctp/endpointola.c | 4 ++++
 2 files changed, 8 insertions(+)

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 62d3cc155809..a5f2835dbe0f 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -999,6 +999,10 @@ 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);
+				if (!chunk->auth_chunk) {
+					chunk->pdiscard = 1;
+					continue;
+				}
 				chunk->auth = 1;
 				continue;
 			}
diff --git a/net/sctp/endpointola.c b/net/sctp/endpointola.c
index dfb1719275db..a15b599b20b7 100644
--- a/net/sctp/endpointola.c
+++ b/net/sctp/endpointola.c
@@ -368,6 +368,10 @@ 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);
+				if (!chunk->auth_chunk) {
+					chunk->pdiscard = 1;
+					continue;
+				}
 				chunk->auth = 1;
 				continue;
 			}
-- 
2.25.1

Thank you for the review and the suggestion.

I agree that using chunk->pdiscard = 1; continue; is better than break;. It avoids stalling the entire inqueue under memory pressure and follows the existing discard convention in SCTP. I will update the patch accordingly.

Thanks,
luoqing

Re: [PATCH net-next v5] sctp: auth: discard auth_chunk when skb_clone fails

Posted by Jakub Kicinski 1 day, 7 hours ago

On Thu, 23 Jul 2026 14:11:07 +0800 luoqing wrote:
> When processing AUTH + COOKIE-ECHO packets, if skb_clone() fails
> due to memory pressure, chunk->auth_chunk is NULL. The original
> code still sets chunk->auth = 1 and continues, leaving the
> COOKIE-ECHO to be processed without a valid auth_chunk for
> deferred verification.
> 
> Discard the AUTH chunk early via pdiscard when skb_clone() fails,
> so that the receive loop can continue processing remaining chunks
> in the inqueue instead of stalling the entire packet.

Please stop sending patches in reply to existing threads.

[PATCH v4 net] sctp: auth: verify auth requirement when auth_chunk is NULL

Posted by luoqing 3 days, 23 hours ago

From: Qing Luo <luoqing@kylinos.cn>

sctp_auth_chunk_verify() returns true unconditionally when
chunk->auth_chunk is NULL, silently skipping authentication.
This is incorrect when:

1. skb_clone() failed in the BH receive path, leaving auth_chunk
   NULL. In sctp_endpoint_bh_rcv() asoc is NULL for new
   connections, so the early sctp_auth_recv_cid() check cannot
   catch this.

2. No AUTH chunk precedes COOKIE-ECHO, so skb_clone() is never
   called and auth_chunk remains NULL.

Fix by checking sctp_auth_recv_cid() when auth_chunk is NULL:
if authentication is required, return false to drop the chunk;
otherwise continue normally.

Fixes: bbd0d59809f9 ("[SCTP]: Implement the receive and verification of AUTH chunk")
Signed-off-by: Qing Luo <luoqing@kylinos.cn>
---
 net/sctp/sm_statefuns.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
index d23d935e128e..89ed618b1de3 100644
--- a/net/sctp/sm_statefuns.c
+++ b/net/sctp/sm_statefuns.c
@@ -642,7 +642,7 @@ static bool sctp_auth_chunk_verify(struct net *net, struct sctp_chunk *chunk,
 	struct sctp_chunk auth;
 
 	if (!chunk->auth_chunk)
-		return true;
+		return !sctp_auth_recv_cid(chunk->chunk_hdr->type, asoc);
 
 	/* SCTP-AUTH:  auth_chunk pointer is only set when the cookie-echo
 	 * is supposed to be authenticated and we have to do delayed
-- 
2.25.1

Re: [PATCH v4 net] sctp: auth: verify auth requirement when auth_chunk is NULL

Posted by Xin Long 2 days, 7 hours ago

On Mon, Jul 20, 2026 at 9:56 PM luoqing <l1138897701@163.com> wrote:
>
> From: Qing Luo <luoqing@kylinos.cn>
>
> sctp_auth_chunk_verify() returns true unconditionally when
> chunk->auth_chunk is NULL, silently skipping authentication.
> This is incorrect when:
>
> 1. skb_clone() failed in the BH receive path, leaving auth_chunk
>    NULL. In sctp_endpoint_bh_rcv() asoc is NULL for new
>    connections, so the early sctp_auth_recv_cid() check cannot
>    catch this.
>
> 2. No AUTH chunk precedes COOKIE-ECHO, so skb_clone() is never
>    called and auth_chunk remains NULL.
>
> Fix by checking sctp_auth_recv_cid() when auth_chunk is NULL:
> if authentication is required, return false to drop the chunk;
> otherwise continue normally.
>
> Fixes: bbd0d59809f9 ("[SCTP]: Implement the receive and verification of AUTH chunk")
> Signed-off-by: Qing Luo <luoqing@kylinos.cn>
> ---
>  net/sctp/sm_statefuns.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c
> index d23d935e128e..89ed618b1de3 100644
> --- a/net/sctp/sm_statefuns.c
> +++ b/net/sctp/sm_statefuns.c
> @@ -642,7 +642,7 @@ static bool sctp_auth_chunk_verify(struct net *net, struct sctp_chunk *chunk,
>         struct sctp_chunk auth;
>
>         if (!chunk->auth_chunk)
> -               return true;
> +               return !sctp_auth_recv_cid(chunk->chunk_hdr->type, asoc);
>
>         /* SCTP-AUTH:  auth_chunk pointer is only set when the cookie-echo
>          * is supposed to be authenticated and we have to do delayed
> --
> 2.25.1
>
Acked-by: Xin Long <lucien.xin@gmail.com>