Re: [RFC] CDC-NCM: avoid overflow in sanity checking

["Bjørn Mork" <bjorn@mork.no>]

Alan Stern <stern@rowland.harvard.edu> writes:

> First, since offset and len are initialized by converting 16- or 32-bit 
> unsigned values from little-endian to cpu-endian, they should be 
> unsigned themselves.
>
> Second, once they are unsigned there is obviously no point in testing 
> whether they are < 0.
>
> Third, if you want to make sure that skb_in's buffer contains the entire 
> interval from offset to offset + len, the proper tests are:
>
> 	if (offset <= skb_in->len && len <= skb_in->len - offset) ...
>
> The first test demonstrates that the start of the interval is in range 
> and the second test demonstrates that the end of the interval is in 
> range.  Furthermore, success of the first test proves that the 
> computation in the second test can't overflow to a negative value.

Thanks.  That detailed explanation makes perfect sense even to me.
Adding the additional offset <= skb_in->len test to Oliver's patch
is sufficient and the best solution.

Only  is that the existing code wants the inverted result:

 	if (offset > skb_in->len || len > skb_in->len - offset) ...

with all values unsigned.

> IMO, working with unsigned values is simpler than working with 
> signed values.  But it does require some discipline to ensure that 
> intermediate computations don't overflow or yield negative values.

And there you point out my problem:  discipline :-)


Bjørn