[FFmpeg-devel] [PATCH 1/3] avformat/evcdec: Avoid nonsense casts

[FFmpeg-devel] [PATCH 1/3] avformat/evcdec: Avoid nonsense casts

[Andreas Rheinhardt]

For uint8_t buf[EVC_NALU_LENGTH_PREFIX_SIZE], &buf still points
to the beginning of buf, but it is of type "pointer to array of
EVC_NALU_LENGTH_PREFIX_SIZE uint8_t" (i.e. pointer arithmetic
would operate on blocks of size EVC_NALU_LENGTH_PREFIX_SIZE).
This is of course a different type than uint8_t*, which is why
there have been casts in evc_read_packet(). But these are unnecessary
if one justs removes the unnecessary address-of operator.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/evcdec.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/libavformat/evcdec.c b/libavformat/evcdec.c
index 73aab6c52f..9886542311 100644
--- a/libavformat/evcdec.c
+++ b/libavformat/evcdec.c
@@ -65,7 +65,7 @@ static int annexb_probe(const AVProbeData *p)
     int nalu_type;
     size_t nalu_size;
     int got_sps = 0, got_pps = 0, got_idr = 0, got_nonidr = 0;
-    unsigned char *bits = (unsigned char *)p->buf;
+    unsigned char *bits = p->buf;
     int bytes_to_read = p->buf_size;
 
     while (bytes_to_read > EVC_NALU_LENGTH_PREFIX_SIZE) {
@@ -159,11 +159,11 @@ static int evc_read_packet(AVFormatContext *s, AVPacket *pkt)
         if (ret < 0)
             return ret;
 
-        ret = avio_read(s->pb, (unsigned char *)&buf, EVC_NALU_LENGTH_PREFIX_SIZE);
+        ret = avio_read(s->pb, buf, EVC_NALU_LENGTH_PREFIX_SIZE);
         if (ret < 0)
             return ret;
 
-        nalu_size = evc_read_nal_unit_length((const uint8_t *)&buf, EVC_NALU_LENGTH_PREFIX_SIZE);
+        nalu_size = evc_read_nal_unit_length(buf, EVC_NALU_LENGTH_PREFIX_SIZE);
         if (!nalu_size || nalu_size > INT_MAX)
             return AVERROR_INVALIDDATA;
 
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 2/3] avformat/evcdec: Check that enough data has been read

[Andreas Rheinhardt]

Fixes potential use of uninitialized values
in evc_read_nal_unit_length().

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/evcdec.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/libavformat/evcdec.c b/libavformat/evcdec.c
index 9886542311..0f464930f7 100644
--- a/libavformat/evcdec.c
+++ b/libavformat/evcdec.c
@@ -162,6 +162,8 @@ static int evc_read_packet(AVFormatContext *s, AVPacket *pkt)
         ret = avio_read(s->pb, buf, EVC_NALU_LENGTH_PREFIX_SIZE);
         if (ret < 0)
             return ret;
+        if (ret != EVC_NALU_LENGTH_PREFIX_SIZE)
+            return AVERROR_INVALIDDATA;
 
         nalu_size = evc_read_nal_unit_length(buf, EVC_NALU_LENGTH_PREFIX_SIZE);
         if (!nalu_size || nalu_size > INT_MAX)
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

[FFmpeg-devel] [PATCH 3/3] avformat/evc: Don't cast const away, avoid loop

[Andreas Rheinhardt]

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
---
 libavformat/evc.h | 17 ++++++-----------
 1 file changed, 6 insertions(+), 11 deletions(-)

diff --git a/libavformat/evc.h b/libavformat/evc.h
index 46b27f7df7..f30831257d 100644
--- a/libavformat/evc.h
+++ b/libavformat/evc.h
@@ -23,16 +23,17 @@
 #define AVFORMAT_EVC_H
 
 #include <stdint.h>
+
+#include "libavutil/intreadwrite.h"
 #include "libavutil/rational.h"
 #include "libavcodec/evc.h"
 #include "avio.h"
 
-static inline int evc_get_nalu_type(const uint8_t *bits, int bits_size)
+static inline int evc_get_nalu_type(const uint8_t *p, int bits_size)
 {
     int unit_type_plus1 = 0;
 
     if (bits_size >= EVC_NALU_HEADER_SIZE) {
-        unsigned char *p = (unsigned char *)bits;
         // forbidden_zero_bit
         if ((p[0] & 0x80) != 0)   // Cannot get bitstream information. Malformed bitstream.
             return -1;
@@ -46,16 +47,10 @@ static inline int evc_get_nalu_type(const uint8_t *bits, int bits_size)
 
 static inline uint32_t evc_read_nal_unit_length(const uint8_t *bits, int bits_size)
 {
-    uint32_t nalu_len = 0;
-
-    if (bits_size >= EVC_NALU_LENGTH_PREFIX_SIZE) {
-        unsigned char *p = (unsigned char *)bits;
-
-        for (int i = 0; i < EVC_NALU_LENGTH_PREFIX_SIZE; i++)
-            nalu_len = (nalu_len << 8) | p[i];
-    }
+    if (bits_size >= EVC_NALU_LENGTH_PREFIX_SIZE)
+        return AV_RB32(bits);
 
-    return nalu_len;
+    return 0;
 }
 
 /**
-- 
2.34.1

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/evcdec: Check that enough data has been read

[James Almer]

On 7/6/2023 6:08 PM, Andreas Rheinhardt wrote:
> Fixes potential use of uninitialized values
> in evc_read_nal_unit_length().
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>   libavformat/evcdec.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/libavformat/evcdec.c b/libavformat/evcdec.c
> index 9886542311..0f464930f7 100644
> --- a/libavformat/evcdec.c
> +++ b/libavformat/evcdec.c
> @@ -162,6 +162,8 @@ static int evc_read_packet(AVFormatContext *s, AVPacket *pkt)
>           ret = avio_read(s->pb, buf, EVC_NALU_LENGTH_PREFIX_SIZE);
>           if (ret < 0)
>               return ret;
> +        if (ret != EVC_NALU_LENGTH_PREFIX_SIZE)
> +            return AVERROR_INVALIDDATA;

There's a ffio_ensure_seekback() for EVC_NALU_LENGTH_PREFIX_SIZE bytes 
immediately before the avio_read() call. Shouldn't that be enough to 
guarantee that much can be read?

Also, you can just pass ret to evc_read_nal_unit_length() below instead 
of adding this check here. It will return an error if it's < 
EVC_NALU_LENGTH_PREFIX_SIZE.

>   
>           nalu_size = evc_read_nal_unit_length(buf, EVC_NALU_LENGTH_PREFIX_SIZE);
>           if (!nalu_size || nalu_size > INT_MAX)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 1/3] avformat/evcdec: Avoid nonsense casts

[James Almer]

On 7/6/2023 6:06 PM, Andreas Rheinhardt wrote:
> For uint8_t buf[EVC_NALU_LENGTH_PREFIX_SIZE], &buf still points
> to the beginning of buf, but it is of type "pointer to array of
> EVC_NALU_LENGTH_PREFIX_SIZE uint8_t" (i.e. pointer arithmetic
> would operate on blocks of size EVC_NALU_LENGTH_PREFIX_SIZE).
> This is of course a different type than uint8_t*, which is why
> there have been casts in evc_read_packet(). But these are unnecessary
> if one justs removes the unnecessary address-of operator.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
> ---
>   libavformat/evcdec.c | 6 +++---
>   1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/libavformat/evcdec.c b/libavformat/evcdec.c
> index 73aab6c52f..9886542311 100644
> --- a/libavformat/evcdec.c
> +++ b/libavformat/evcdec.c
> @@ -65,7 +65,7 @@ static int annexb_probe(const AVProbeData *p)
>       int nalu_type;
>       size_t nalu_size;
>       int got_sps = 0, got_pps = 0, got_idr = 0, got_nonidr = 0;
> -    unsigned char *bits = (unsigned char *)p->buf;
> +    unsigned char *bits = p->buf;
>       int bytes_to_read = p->buf_size;
>   
>       while (bytes_to_read > EVC_NALU_LENGTH_PREFIX_SIZE) {
> @@ -159,11 +159,11 @@ static int evc_read_packet(AVFormatContext *s, AVPacket *pkt)
>           if (ret < 0)
>               return ret;
>   
> -        ret = avio_read(s->pb, (unsigned char *)&buf, EVC_NALU_LENGTH_PREFIX_SIZE);
> +        ret = avio_read(s->pb, buf, EVC_NALU_LENGTH_PREFIX_SIZE);
>           if (ret < 0)
>               return ret;
>   
> -        nalu_size = evc_read_nal_unit_length((const uint8_t *)&buf, EVC_NALU_LENGTH_PREFIX_SIZE);
> +        nalu_size = evc_read_nal_unit_length(buf, EVC_NALU_LENGTH_PREFIX_SIZE);
>           if (!nalu_size || nalu_size > INT_MAX)
>               return AVERROR_INVALIDDATA;

LGTM.
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/evcdec: Check that enough data has been read

[James Almer]

On 7/6/2023 10:14 PM, James Almer wrote:
> On 7/6/2023 6:08 PM, Andreas Rheinhardt wrote:
>> Fixes potential use of uninitialized values
>> in evc_read_nal_unit_length().
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>   libavformat/evcdec.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/libavformat/evcdec.c b/libavformat/evcdec.c
>> index 9886542311..0f464930f7 100644
>> --- a/libavformat/evcdec.c
>> +++ b/libavformat/evcdec.c
>> @@ -162,6 +162,8 @@ static int evc_read_packet(AVFormatContext *s, 
>> AVPacket *pkt)
>>           ret = avio_read(s->pb, buf, EVC_NALU_LENGTH_PREFIX_SIZE);
>>           if (ret < 0)
>>               return ret;
>> +        if (ret != EVC_NALU_LENGTH_PREFIX_SIZE)
>> +            return AVERROR_INVALIDDATA;
> 
> There's a ffio_ensure_seekback() for EVC_NALU_LENGTH_PREFIX_SIZE bytes 
> immediately before the avio_read() call. Shouldn't that be enough to 
> guarantee that much can be read?
> 
> Also, you can just pass ret to evc_read_nal_unit_length() below instead 
> of adding this check here. It will return an error if it's < 
> EVC_NALU_LENGTH_PREFIX_SIZE.

Oh, my bad, i was looking at the function of the same name in 
libavcodec/evc_parse.h

The function in evc.h could be changed to also use the same check as the 
one the evc_parse.h version alongside the other change you're doing in 
patch 3/3.

> 
>>           nalu_size = evc_read_nal_unit_length(buf, 
>> EVC_NALU_LENGTH_PREFIX_SIZE);
>>           if (!nalu_size || nalu_size > INT_MAX)
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/evcdec: Check that enough data has been read

[Andreas Rheinhardt]

James Almer:
> On 7/6/2023 6:08 PM, Andreas Rheinhardt wrote:
>> Fixes potential use of uninitialized values
>> in evc_read_nal_unit_length().
>>
>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>> ---
>>   libavformat/evcdec.c | 2 ++
>>   1 file changed, 2 insertions(+)
>>
>> diff --git a/libavformat/evcdec.c b/libavformat/evcdec.c
>> index 9886542311..0f464930f7 100644
>> --- a/libavformat/evcdec.c
>> +++ b/libavformat/evcdec.c
>> @@ -162,6 +162,8 @@ static int evc_read_packet(AVFormatContext *s,
>> AVPacket *pkt)
>>           ret = avio_read(s->pb, buf, EVC_NALU_LENGTH_PREFIX_SIZE);
>>           if (ret < 0)
>>               return ret;
>> +        if (ret != EVC_NALU_LENGTH_PREFIX_SIZE)
>> +            return AVERROR_INVALIDDATA;
> 
> There's a ffio_ensure_seekback() for EVC_NALU_LENGTH_PREFIX_SIZE bytes
> immediately before the avio_read() call. Shouldn't that be enough to
> guarantee that much can be read?
> 

ffio_ensure_seekback() ensures that the read buffer is big enough so
that reading EVC_NALU_LENGTH_PREFIX_SIZE bytes does not lead to a reset
of the buffer; it does not imply that the buffer already contains
EVC_NALU_LENGTH_PREFIX_SIZE bytes. In fact, there is not a single
codepath in ffio_ensure_seekback() that actually reads further input.
(If EVC_NALU_LENGTH_PREFIX_SIZE bytes are not available in the buffer,
then the buf_size <= s->buffer_size codepath will likely be taken in the
non-seekable case (in the seekable case, ffio_ensure_seekback() does
even less, namely nothing).)

> Also, you can just pass ret to evc_read_nal_unit_length() below instead
> of adding this check here. It will return an error if it's <
> EVC_NALU_LENGTH_PREFIX_SIZE.
> 

It will actually return 0 which the caller will transform into an error.
I do not want to rely on this behaviour.
(Why did you add two inline functions of the same name in different evc
headers?)

>>             nalu_size = evc_read_nal_unit_length(buf,
>> EVC_NALU_LENGTH_PREFIX_SIZE);
>>           if (!nalu_size || nalu_size > INT_MAX)

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/evcdec: Check that enough data has been read

[Andreas Rheinhardt]

James Almer:
> On 7/6/2023 10:14 PM, James Almer wrote:
>> On 7/6/2023 6:08 PM, Andreas Rheinhardt wrote:
>>> Fixes potential use of uninitialized values
>>> in evc_read_nal_unit_length().
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>>> ---
>>>   libavformat/evcdec.c | 2 ++
>>>   1 file changed, 2 insertions(+)
>>>
>>> diff --git a/libavformat/evcdec.c b/libavformat/evcdec.c
>>> index 9886542311..0f464930f7 100644
>>> --- a/libavformat/evcdec.c
>>> +++ b/libavformat/evcdec.c
>>> @@ -162,6 +162,8 @@ static int evc_read_packet(AVFormatContext *s,
>>> AVPacket *pkt)
>>>           ret = avio_read(s->pb, buf, EVC_NALU_LENGTH_PREFIX_SIZE);
>>>           if (ret < 0)
>>>               return ret;
>>> +        if (ret != EVC_NALU_LENGTH_PREFIX_SIZE)
>>> +            return AVERROR_INVALIDDATA;
>>
>> There's a ffio_ensure_seekback() for EVC_NALU_LENGTH_PREFIX_SIZE bytes
>> immediately before the avio_read() call. Shouldn't that be enough to
>> guarantee that much can be read?
>>
>> Also, you can just pass ret to evc_read_nal_unit_length() below
>> instead of adding this check here. It will return an error if it's <
>> EVC_NALU_LENGTH_PREFIX_SIZE.
> 
> Oh, my bad, i was looking at the function of the same name in
> libavcodec/evc_parse.h
> 
> The function in evc.h could be changed to also use the same check as the
> one the evc_parse.h version alongside the other change you're doing in
> patch 3/3.
> 

These functions already do the same (except for the log message); they
both return 0 if not enough data is available. The return value would
need to be int64_t if one wanted to return both error codes and lengths
via it. I don't really see the advantage of this.

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/evcdec: Check that enough data has been read

[James Almer]

On 7/6/2023 10:30 PM, Andreas Rheinhardt wrote:
> James Almer:
>> On 7/6/2023 10:14 PM, James Almer wrote:
>>> On 7/6/2023 6:08 PM, Andreas Rheinhardt wrote:
>>>> Fixes potential use of uninitialized values
>>>> in evc_read_nal_unit_length().
>>>>
>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>>>> ---
>>>>    libavformat/evcdec.c | 2 ++
>>>>    1 file changed, 2 insertions(+)
>>>>
>>>> diff --git a/libavformat/evcdec.c b/libavformat/evcdec.c
>>>> index 9886542311..0f464930f7 100644
>>>> --- a/libavformat/evcdec.c
>>>> +++ b/libavformat/evcdec.c
>>>> @@ -162,6 +162,8 @@ static int evc_read_packet(AVFormatContext *s,
>>>> AVPacket *pkt)
>>>>            ret = avio_read(s->pb, buf, EVC_NALU_LENGTH_PREFIX_SIZE);
>>>>            if (ret < 0)
>>>>                return ret;
>>>> +        if (ret != EVC_NALU_LENGTH_PREFIX_SIZE)
>>>> +            return AVERROR_INVALIDDATA;
>>>
>>> There's a ffio_ensure_seekback() for EVC_NALU_LENGTH_PREFIX_SIZE bytes
>>> immediately before the avio_read() call. Shouldn't that be enough to
>>> guarantee that much can be read?
>>>
>>> Also, you can just pass ret to evc_read_nal_unit_length() below
>>> instead of adding this check here. It will return an error if it's <
>>> EVC_NALU_LENGTH_PREFIX_SIZE.
>>
>> Oh, my bad, i was looking at the function of the same name in
>> libavcodec/evc_parse.h
>>
>> The function in evc.h could be changed to also use the same check as the
>> one the evc_parse.h version alongside the other change you're doing in
>> patch 3/3.
>>
> 
> These functions already do the same (except for the log message); they
> both return 0 if not enough data is available. The return value would
> need to be int64_t if one wanted to return both error codes and lengths
> via it. I don't really see the advantage of this.

Fair enough. Then what about ffio_ensure_seekback()? Should that not 
guarantee at least EVC_NALU_LENGTH_PREFIX_SIZE bytes are buffered, thus 
readable and seekable backwards?
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/evcdec: Check that enough data has been read

[Andreas Rheinhardt]

James Almer:
> On 7/6/2023 10:30 PM, Andreas Rheinhardt wrote:
>> James Almer:
>>> On 7/6/2023 10:14 PM, James Almer wrote:
>>>> On 7/6/2023 6:08 PM, Andreas Rheinhardt wrote:
>>>>> Fixes potential use of uninitialized values
>>>>> in evc_read_nal_unit_length().
>>>>>
>>>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>>>>> ---
>>>>>    libavformat/evcdec.c | 2 ++
>>>>>    1 file changed, 2 insertions(+)
>>>>>
>>>>> diff --git a/libavformat/evcdec.c b/libavformat/evcdec.c
>>>>> index 9886542311..0f464930f7 100644
>>>>> --- a/libavformat/evcdec.c
>>>>> +++ b/libavformat/evcdec.c
>>>>> @@ -162,6 +162,8 @@ static int evc_read_packet(AVFormatContext *s,
>>>>> AVPacket *pkt)
>>>>>            ret = avio_read(s->pb, buf, EVC_NALU_LENGTH_PREFIX_SIZE);
>>>>>            if (ret < 0)
>>>>>                return ret;
>>>>> +        if (ret != EVC_NALU_LENGTH_PREFIX_SIZE)
>>>>> +            return AVERROR_INVALIDDATA;
>>>>
>>>> There's a ffio_ensure_seekback() for EVC_NALU_LENGTH_PREFIX_SIZE bytes
>>>> immediately before the avio_read() call. Shouldn't that be enough to
>>>> guarantee that much can be read?
>>>>
>>>> Also, you can just pass ret to evc_read_nal_unit_length() below
>>>> instead of adding this check here. It will return an error if it's <
>>>> EVC_NALU_LENGTH_PREFIX_SIZE.
>>>
>>> Oh, my bad, i was looking at the function of the same name in
>>> libavcodec/evc_parse.h
>>>
>>> The function in evc.h could be changed to also use the same check as the
>>> one the evc_parse.h version alongside the other change you're doing in
>>> patch 3/3.
>>>
>>
>> These functions already do the same (except for the log message); they
>> both return 0 if not enough data is available. The return value would
>> need to be int64_t if one wanted to return both error codes and lengths
>> via it. I don't really see the advantage of this.
> 
> Fair enough. Then what about ffio_ensure_seekback()? Should that not
> guarantee at least EVC_NALU_LENGTH_PREFIX_SIZE bytes are buffered, thus
> readable and seekable backwards?

No. It is just supposed to ensure that if one reads
EVC_NALU_LENGTH_PREFIX_SIZE bytes, one can seek back and "unread" them.
See also my earlier mail.

- Andreas

_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

Re: [FFmpeg-devel] [PATCH 2/3] avformat/evcdec: Check that enough data has been read

[James Almer]

On 7/6/2023 10:24 PM, Andreas Rheinhardt wrote:
> James Almer:
>> On 7/6/2023 6:08 PM, Andreas Rheinhardt wrote:
>>> Fixes potential use of uninitialized values
>>> in evc_read_nal_unit_length().
>>>
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>>> ---
>>>    libavformat/evcdec.c | 2 ++
>>>    1 file changed, 2 insertions(+)
>>>
>>> diff --git a/libavformat/evcdec.c b/libavformat/evcdec.c
>>> index 9886542311..0f464930f7 100644
>>> --- a/libavformat/evcdec.c
>>> +++ b/libavformat/evcdec.c
>>> @@ -162,6 +162,8 @@ static int evc_read_packet(AVFormatContext *s,
>>> AVPacket *pkt)
>>>            ret = avio_read(s->pb, buf, EVC_NALU_LENGTH_PREFIX_SIZE);
>>>            if (ret < 0)
>>>                return ret;
>>> +        if (ret != EVC_NALU_LENGTH_PREFIX_SIZE)
>>> +            return AVERROR_INVALIDDATA;
>>
>> There's a ffio_ensure_seekback() for EVC_NALU_LENGTH_PREFIX_SIZE bytes
>> immediately before the avio_read() call. Shouldn't that be enough to
>> guarantee that much can be read?
>>
> 
> ffio_ensure_seekback() ensures that the read buffer is big enough so
> that reading EVC_NALU_LENGTH_PREFIX_SIZE bytes does not lead to a reset
> of the buffer; it does not imply that the buffer already contains
> EVC_NALU_LENGTH_PREFIX_SIZE bytes. In fact, there is not a single
> codepath in ffio_ensure_seekback() that actually reads further input.
> (If EVC_NALU_LENGTH_PREFIX_SIZE bytes are not available in the buffer,
> then the buf_size <= s->buffer_size codepath will likely be taken in the
> non-seekable case (in the seekable case, ffio_ensure_seekback() does
> even less, namely nothing).)
> 
>> Also, you can just pass ret to evc_read_nal_unit_length() below instead
>> of adding this check here. It will return an error if it's <
>> EVC_NALU_LENGTH_PREFIX_SIZE.
>>
> 
> It will actually return 0 which the caller will transform into an error.
> I do not want to rely on this behaviour.

Ok, patch LGTM then.

> (Why did you add two inline functions of the same name in different evc
> headers?)

If i recall correctly, because the probe function must not call av_log.

> 
>>>              nalu_size = evc_read_nal_unit_length(buf,
>>> EVC_NALU_LENGTH_PREFIX_SIZE);
>>>            if (!nalu_size || nalu_size > INT_MAX)
> 
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".