Re: [PATCH v2 2/2] net: sctp: auth: Fix safety issue when skb_clone fails in auth_chunk handling

[Xin Long]

On Fri, Jul 17, 2026 at 11:01 AM Xin Long <lucien.xin@xxxxxxxxx> wrote:
>
> On Thu, Jul 16, 2026 at 6:05 AM luoqing <l1138897701@xxxxxxx> wrote:
> >
> > From: luoqing <luoqing@xxxxxxxxxx>
> >
> > 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@xxxxxxxxxx>
> > ---
> >  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.