Hi, >> question in c->key_meshing case. And the concern is that it will >> incorrectly handle overflows. Is it correctly understood? In such case > > > Exactly, this bug also presented. > >> In this case it would be possible to declare count as >> platform-independent [and therefore preferred] unsigned int. Well, one >> can even do unsigned short...
While 'short' would do, it's not really optimal choice, because its treatment can take more resources on some platforms. Not really critical, but short wins you exactly nothing either way. You don't even save memory, because structure fields are maintained aligned so that e.g. "int,short,int" takes as much space as "int,int,int". I mentioned 'short' mostly for sake of *illuminating* the problem. As for signed vs. unsigned. signed%1024 takes more instruction to compute, so unsigned should be preferred (in addition to that byte count is arguably naturally unsigned). Not critical either... > IMHO, platform-independent type 'short' better. This type needed short and > fast test cases. > > Attachment "mac.121219.patch" (3 KB), change from 121214-1: > - Change type of 'count' member from 'unsigned long long'/'off_t' to > 'short'; There is problem with overflow in another part of the suggested patch, namely in "if (c->count<8 && c->bytes_left)" in gost_imit_final. Indeed, count is incremented by 8 *unconditionally* while ceiled by 2040 only in key_meshing case. Note that declaring it short makes it worse (and signed is much more worse that unsigend:-) Either way, correct solution is to even ceil unconditionally. Shall we settle for http://cvs.openssl.org/chngview?cn=23135? > Attachment "test2814789.121219.patch" (63 KB), change from 121214-1: > - Add test cases for CFB and CNT modes with clear text length 5 MB and, > optional, 4 GB. Once we settle on solution, it will be committed. Cheers. ______________________________________________________________________ OpenSSL Project http://www.openssl.org Development Mailing List [email protected] Automated List Manager [email protected]
