Alan Stern <stern@xxxxxxxxxxxxxxxxxxx> 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
