I agree that this is not a security issue if key_len can never get large. So how about just removing the overflow check at all? - xi On Nov 28, 2011, at 10:45 AM, Vladislav Yasevich wrote: > > Hmm. Yes, this is a more correct check. > > Acked-by: Vlad Yasevich <vladislav.yasevich@xxxxxx> > > > However, I don't think this is a security issue. As I've written before, this function is > called from 2 places: > > 1) setsockopt() code path > > 2) sctp_auth_asoc_set_secret() code path > > In case (1), sca_keylength is never going to exceed 65535 since it's > bounded by a u16 from the user api. As such, The integer promotion will > not impact anything and the malloc() will never overflow. > > In case (2), sca_keylength is computed based on the key the user provided > (MAX_USHORT) and the combination of protocol negotiated data where that > combination has a max size of 3 * MAX_USHORT (see sctp_auth_make_key_vector()). > So, even this case, our maximum key length can be 4* MAX_USHORT which still > will always be below MAX_INT and will not overflow. > > So, I don't think there is big security consideration here, just a bad > check that just happens to always work. > > -vlad -- To unsubscribe from this list: send the line "unsubscribe linux-sctp" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
