Hello! It seems the patch may lead to NULL pointer dereference. 1. sctp_sf_violation_chunk() calls sctp_sf_violation() with asoc arg equal to NULL. static enum sctp_disposition sctp_sf_violation_chunk( ... { ... if (!asoc) return sctp_sf_violation(net, ep, asoc, type, arg, commands); ... 2. Newly added code of sctp_sf_violation() calls to sctp_vtag_verify() with asoc arg equal to NULL. enum sctp_disposition sctp_sf_violation(struct net *net, ... { struct sctp_chunk *chunk = arg; if (!sctp_vtag_verify(chunk, asoc)) return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); ... 3. sctp_vtag_verify() dereferences asoc without any check. /* Check VTAG of the packet matches the sender's own tag. */ static inline int sctp_vtag_verify(const struct sctp_chunk *chunk, const struct sctp_association *asoc) { /* RFC 2960 Sec 8.5 When receiving an SCTP packet, the endpoint * MUST ensure that the value in the Verification Tag field of * the received SCTP packet matches its own Tag. If the received * Verification Tag value does not match the receiver's own * tag value, the receiver shall silently discard the packet... */ if (ntohl(chunk->sctp_hdr->vtag) != asoc->c.my_vtag) return 0; Found by Linux Verification Center (linuxtesting.org) with SVACE tool. -- Best regards, Alexey Khoroshilov Linux Verification Center, ISPRAS On 01.11.2021 12:17, Greg Kroah-Hartman wrote: > From: Xin Long <lucien.xin@xxxxxxxxx> > > [ Upstream commit aa0f697e45286a6b5f0ceca9418acf54b9099d99 ] > > sctp_sf_violation() is called when processing HEARTBEAT_ACK chunk > in cookie_wait state, and some other places are also using it. > > The vtag in the chunk's sctphdr should be verified, otherwise, as > later in chunk length check, it may send abort with the existent > asoc's vtag, which can be exploited by one to cook a malicious > chunk to terminate a SCTP asoc. > > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2") > Signed-off-by: Xin Long <lucien.xin@xxxxxxxxx> > Acked-by: Marcelo Ricardo Leitner <marcelo.leitner@xxxxxxxxx> > Signed-off-by: Jakub Kicinski <kuba@xxxxxxxxxx> > Signed-off-by: Sasha Levin <sashal@xxxxxxxxxx> > --- > net/sctp/sm_statefuns.c | 3 +++ > 1 file changed, 3 insertions(+) > > diff --git a/net/sctp/sm_statefuns.c b/net/sctp/sm_statefuns.c > index 0cfbf6046bf8..324c0222d9e6 100644 > --- a/net/sctp/sm_statefuns.c > +++ b/net/sctp/sm_statefuns.c > @@ -4549,6 +4549,9 @@ enum sctp_disposition sctp_sf_violation(struct net *net, > { > struct sctp_chunk *chunk = arg; > > + if (!sctp_vtag_verify(chunk, asoc)) > + return sctp_sf_pdiscard(net, ep, asoc, type, arg, commands); > + > /* Make sure that the chunk has a valid length. */ > if (!sctp_chunk_length_valid(chunk, sizeof(struct sctp_chunkhdr))) > return sctp_sf_violation_chunklen(net, ep, asoc, type, arg, >