Re: [PATCH v2 2/2] net: sctp: auth: Fix safety issue when skb_clone fails in auth_chunk handling
Xin Long <[email protected]> Fri, 17 Jul 2026 11:22:09 -0400
| Newsgroups | org.kernel.vger.linux-sctp,org.kernel.vger.linux-kernel,org.kernel.vger.netdev |
|---|---|
| Message-ID | <CADvbK_e7rFqpWzDLvP2KPucbFzr1CwX9JotMoBE=HK4H_uOLpQ@mail.gmail.com> |
On Fri, Jul 17, 2026 at 11:01=E2=80=AFAM Xin Long <[email protected]> wr= ote: > > On Thu, Jul 16, 2026 at 6:05=E2=80=AFAM luoqing <[email protected]> wro= te: > > > > From: luoqing <[email protected]> > > > > 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 =3D 1 when skb_clone succeeds. > > > > Fixes: 59d8d4434f429b ("sctp: delay the authentication for the duplicat= ed cookie-echo chunk") > > > > Signed-off-by: luoqing <[email protected]> > > --- > > 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 *w= ork) > > if (next_hdr->type =3D=3D SCTP_CID_COOKIE_ECHO)= { > > chunk->auth_chunk =3D skb_clone(chunk->= skb, > > GFP_ATOMI= C); > > - chunk->auth =3D 1; > > + if (chunk->auth_chunk) > > + chunk->auth =3D 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 =3D=3D SCTP_CID_COOKIE_ECHO)= { > > chunk->auth_chunk =3D skb_clone(chunk->= skb, > > GFP_ATO= MIC); > > - chunk->auth =3D 1; > > + if (chunk->auth_chunk) > > + chunk->auth =3D 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 b= e > processed with chunk->auth_chunk =3D=3D NULL. > > 2. [sashiko-claude]: > > When the association requires authentication for the COOKIE-ECHO chunk, b= ut > the packet does not contain an AUTH chunk, skb_clone() is never called an= d > 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. Th= e > COOKIE-ECHO chunk will still be processed with chunk->auth_chunk =3D=3D N= ULL. > > A better fix would be: > > Add a check in sctp_auth_chunk_verify() at the point where the COOKIE-ECH= O > 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.