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@gmail.com
[ 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@gmail.com Acked-by: Marcelo Ricardo Leitner marcelo.leitner@gmail.com Signed-off-by: Jakub Kicinski kuba@kernel.org Signed-off-by: Sasha Levin sashal@kernel.org
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,