[FFmpeg-devel] ADPCM task (was Re: files in incoming)

[Stefan Gehrer]

M?ns Rullg?rd wrote:
> Stefan Gehrer <stefan.gehrer at gmx.de> writes:
> 
>> Reimar D?ffinger wrote:
>>> On Fri, Jan 30, 2009 at 06:55:04PM +0100, Stefan Gehrer wrote:
>>>> Reimar D?ffinger wrote:
>>>>> On Fri, Jan 30, 2009 at 08:06:04AM +0100, Stefan Gehrer wrote:
>>>>>> @@ -1303,6 +1304,7 @@
>>>>>>              srcC  = src + (avctx->channels-channel) * 4;
>>>>>>              srcC += (big_endian ? bytestream_get_be32(&src)
>>>>>>                                  : bytestream_get_le32(&src));
>>>>>> +            if ((srcC > src_end - 4) || (srcC < src)) break;
>>>>> Unfortunately no, a C compiler is allowed to assume that pointer
>>>>> operations will never overflow, thus removing the (srcC < src) check.
>>>> Interesting. Do you have a source where I can read that up?
>>>> And if the answer is ANSI C / ISO 9899, maybe a more specific hint?
>>> Well, the way I put it is the reality as gcc handles it, the standard is
>>> far more restrictive, quoting the C99 standardi, section 6.5.6.8:
>>>> If both the pointer
>>>> operand and the result point to elements of the same array object, or
>>>> one past the last
>>>> element of the array object, the evaluation shall not produce an
>>>> overflow; otherwise, the
>>>> behavior is undefined.
>>> Emphasis on "undefined", which is about the worst case you can
>>> encounter.
>>> So even as soon as you just add a value that is larger than
>>> the array size to a pointer anything may happen just going by this...
>>> In case of gcc, "only" an important half of your security check goes
>>> missing.
>> Hmm, I am not entirely convinced. For me that rather reads
>> "if you do your pointer arithmetics nicely within an array, their
>> will be no overflow. If you do it beyond an array, there may
>> or may not be an overflow."
>> That means after our calculation of srcC there might have
>> been an overflow, as it is not pointing to an element of an array
>> object, therefore a check on srcC being smaller than src is a valid
>> check that can not be omitted.
>> But of course I am happy to be corrected ...
> 
> Pointer arithmetic resulting in a pointer not within or immediately
> beyond the array has undefined behaviour.  This includes doing nothing
> at all.  For srcC < src to be true, an overflow must have occurred.
> The standard states that arithmetic within the defined bounds does not
> overflow, hence an operation with undefined behaviour must have been
> performed.  Whatever that operation was, it isn't required to overflow
> (it's undefined), and the compiler is free to do anything, including
> dropping the check.

Ok, so the last patch in this thread (attached again for clarity) seems
to be the right thing. I will apply soon as I think it is better than
leaving that pointer (which depends on input data) unchecked.

Stefan

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: adpcmsrc2.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090202/da98421e/attachment.asc>