Re: [Lksctp-developers] [PATCH 2/3] sctp: fix integer overflow when

[Vlad Yasevich <vladislav.yasevich@hp.com>]

Andrei Pelinescu-Onciul wrote:
> On Nov 12, 2009 at 11:29, Vlad Yasevich <vladislav.yasevich@hp.com> wrote:
>>
>> Andrei Pelinescu-Onciul wrote:
>>> When setting the autoclose timeout in jiffies there is a possible
>>> integer overflow if the value in seconds is very large
>>> (e.g. for 2^22 s with HZ\x1024). The problem appears even on
>>> 64-bit due to the integer promotion rules. The fix is just a cast
>>>  to unsigned long.
>>>
>>> Signed-off-by: Andrei Pelinescu-Onciul <andrei@iptel.org>
>>> ---
>>>  net/sctp/associola.c |    2 +-
>>>  1 files changed, 1 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/net/sctp/associola.c b/net/sctp/associola.c
>>> index 525864b..7f69f4d 100644
>>> --- a/net/sctp/associola.c
>>> +++ b/net/sctp/associola.c
>>> @@ -166,7 +166,7 @@ static struct sctp_association *sctp_association_init(struct sctp_association *a
>>>  	asoc->timeouts[SCTP_EVENT_TIMEOUT_HEARTBEAT] = 0;
>>>  	asoc->timeouts[SCTP_EVENT_TIMEOUT_SACK] = asoc->sackdelay;
>>>  	asoc->timeouts[SCTP_EVENT_TIMEOUT_AUTOCLOSE] >>> -		sp->autoclose * HZ;
>>> +		(unsigned long)sp->autoclose * HZ;
>>>  
>>>  	/* Initilizes the timers */
>>>  	for (i = SCTP_EVENT_TIMEOUT_NONE; i < SCTP_NUM_TIMEOUT_TYPES; ++i)
>> This becomes unnecessary with Patch 3.
> 
> I don't think so. Patch 3 makes sure 
> sp->autoclose <= (MAX_SCHEDULE_TIMEOUT / HZ).
> On 64 bits this is always true, because autoclose is u32,
> MAX_SCHEDULE_TIMEOUT is LONG_MAX (2^63-1) and HZ <= 1024, so on 64 bits
> patch 3 will not do anything and sp->autoclose * HZ can still overflow
> an int.
> E.g.: autoclose= 2^22, HZ\x1024.
> 2^22 < (2^63-1)/1024 => patch 3 does not change autoclose
> However  2^22 * 1024 = 2^32 which is > UINT_MAX =>
> asoc->timeouts[SCTP_EVENT_TIMEOUT_AUTOCLOSE] will be set to
> (uint)2^32 = 0!
> 

Ok.  So, we'll change patch 3 to do:

if (sp->autoclose * HZ > MAX_SCHEDULE_TIMEOUT)
	sp->autoclose = MAX_SCHEDULE_TIMEOUT/HZ;

-vlad


> 
> Andrei
> 
> ------------------------------------------------------------------------------
> Let Crystal Reports handle the reporting - Free Crystal Reports 2008 30-Day 
> trial. Simplify your report design, integration and deployment - and focus on 
> what you do best, core application coding. Discover what's new with
> Crystal Reports now.  http://p.sf.net/sfu/bobj-july
> _______________________________________________
> Lksctp-developers mailing list
> Lksctp-developers@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lksctp-developers
>