From 7cc08b55fc476a9474e4dc9da41071b5dc2b406e Mon Sep 17 00:00:00 2001 From: Wei Yongjun Date: Tue, 5 Feb 2008 03:03:06 -0800 Subject: [PATCH] [SCTP]: Fix kernel panic while received AUTH chunk with BAD shared key identifier If SCTP-AUTH is enabled, received AUTH chunk with BAD shared key identifier will cause kernel panic. Test as following: step1: enabled /proc/sys/net/sctp/auth_enable step 2: connect to SCTP server with auth capable. Association is established between endpoints. Then send a AUTH chunk with a bad shareid, SCTP server will kernel panic after received that AUTH chunk. SCTP client SCTP server INIT ----------> (with auth capable) <---------- INIT-ACK (with auth capable) COOKIE-ECHO ----------> <---------- COOKIE-ACK AUTH ----------> AUTH chunk is like this: AUTH chunk Chunk type: AUTH (15) Chunk flags: 0x00 Chunk length: 28 Shared key identifier: 10 HMAC identifier: SHA-1 (1) HMAC: 0000000000000000000000000000000000000000 The assignment of NULL to key can safely be removed, since key_for_each (which is just list_for_each_entry under the covers does an initial assignment to key anyway). If the endpoint_shared_keys list is empty, or if the key_id being requested does not exist, the function as it currently stands returns the actuall list_head (in this case endpoint_shared_keys. Since that list_head isn't surrounded by an actuall data structure, the last iteration through list_for_each_entry will do a container_of on key, and we wind up returning a bogus pointer, instead of NULL, as we should. > Neil Horman wrote: >> On Tue, Jan 22, 2008 at 05:29:20PM +0900, Wei Yongjun wrote: >> >> FWIW, Ack from me. The assignment of NULL to key can safely be >> removed, since >> key_for_each (which is just list_for_each_entry under the covers does >> an initial >> assignment to key anyway). >> If the endpoint_shared_keys list is empty, or if the key_id being >> requested does >> not exist, the function as it currently stands returns the actuall >> list_head (in >> this case endpoint_shared_keys. Since that list_head isn't >> surrounded by an >> actuall data structure, the last iteration through >> list_for_each_entry will do a >> container_of on key, and we wind up returning a bogus pointer, >> instead of NULL, >> as we should. Wei's patch corrects that. >> >> Regards >> Neil >> >> Acked-by: Neil Horman >> > > Yep, the patch is correct. > > Acked-by: Vlad Yasevich > > -vlad > Signed-off-by: Wei Yongjun Acked-by: Neil Horman Acked-by: Vlad Yasevich Signed-off-by: David S. Miller --- net/sctp/auth.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/net/sctp/auth.c b/net/sctp/auth.c index 97e6ebd1450..ae367c82e51 100644 --- a/net/sctp/auth.c +++ b/net/sctp/auth.c @@ -420,15 +420,15 @@ struct sctp_shared_key *sctp_auth_get_shkey( const struct sctp_association *asoc, __u16 key_id) { - struct sctp_shared_key *key = NULL; + struct sctp_shared_key *key; /* First search associations set of endpoint pair shared keys */ key_for_each(key, &asoc->endpoint_shared_keys) { if (key->key_id == key_id) - break; + return key; } - return key; + return NULL; } /* -- 2.41.1