summaryrefslogtreecommitdiffstats
path: root/net
diff options
context:
space:
mode:
authorWei Yongjun <yjwei@cn.fujitsu.com>2008-02-05 03:03:06 -0800
committerDavid S. Miller <davem@davemloft.net>2008-02-05 03:03:06 -0800
commit7cc08b55fc476a9474e4dc9da41071b5dc2b406e (patch)
tree0fce947d2feb2242d644f800c69965b944733d0e /net
parentd2f19fa13ee5e78d4195a771f8f1ff7d42a80740 (diff)
downloadlinux-7cc08b55fc476a9474e4dc9da41071b5dc2b406e.tar.bz2
[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 <nhorman@tuxdriver.com> >> > > Yep, the patch is correct. > > Acked-by: Vlad Yasevich <vladislav.yasevich@hp.com> > > -vlad > Signed-off-by: Wei Yongjun <yjwei@cn.fujitsu.com> Acked-by: Neil Horman <nhorman@tuxdriver.com> Acked-by: Vlad Yasevich <vladislav.yasevich@hp.com> Signed-off-by: David S. Miller <davem@davemloft.net>
Diffstat (limited to 'net')
-rw-r--r--net/sctp/auth.c6
1 files changed, 3 insertions, 3 deletions
diff --git a/net/sctp/auth.c b/net/sctp/auth.c
index 97e6ebd14500..ae367c82e512 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;
}
/*