diff mbox

[FFmpeg-devel,04/37] avformat/matroskadec: Don't zero unnecessarily

Message ID 20190516223018.30827-5-andreas.rheinhardt@gmail.com
State Accepted
Headers show

Commit Message

Andreas Rheinhardt May 16, 2019, 10:29 p.m. UTC
It is only necessary to zero the initial allocated memory used to store
the size of laced frames if the block used Xiph lacing. Otherwise no
unintialized data was ever used, so use av_malloc instead of av_mallocz.

Also use the correct type for the allocations.

Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
---
I checked this via the --enable-memory-poisoning compile option. It
yielded identical output to patched version without this option as well
as to current git head; I used Alexander Noe's AVI-Mux GUI 
(http://www.alexander-noe.com/video/amg/) to create files using Xiph
lacing to test this on.
I unfortunately couldn't make Clang's MemorySanitizer or its
AddressSanitizer run under MinGW64.
 libavformat/matroskadec.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

Comments

James Almer June 25, 2019, 12:04 a.m. UTC | #1
On 5/16/2019 7:29 PM, Andreas Rheinhardt wrote:
> It is only necessary to zero the initial allocated memory used to store
> the size of laced frames if the block used Xiph lacing. Otherwise no
> unintialized data was ever used, so use av_malloc instead of av_mallocz.
> 
> Also use the correct type for the allocations.
> 
> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@gmail.com>
> ---
> I checked this via the --enable-memory-poisoning compile option. It
> yielded identical output to patched version without this option as well
> as to current git head; I used Alexander Noe's AVI-Mux GUI 
> (http://www.alexander-noe.com/video/amg/) to create files using Xiph
> lacing to test this on.
> I unfortunately couldn't make Clang's MemorySanitizer or its
> AddressSanitizer run under MinGW64.
>  libavformat/matroskadec.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
> index 3b8ddc5ecb..4dd933ef74 100644
> --- a/libavformat/matroskadec.c
> +++ b/libavformat/matroskadec.c
> @@ -2814,7 +2814,7 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
>  
>      if (!type) {
>          *laces    = 1;
> -        *lace_buf = av_mallocz(sizeof(int));
> +        *lace_buf = av_malloc(sizeof(**lace_buf));
>          if (!*lace_buf)
>              return AVERROR(ENOMEM);
>  
> @@ -2826,7 +2826,7 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
>      *laces    = *data + 1;
>      data     += 1;
>      size     -= 1;
> -    lace_size = av_mallocz(*laces * sizeof(int));
> +    lace_size = av_malloc(*laces * sizeof(*lace_size));

Changed this to av_malloc_array() while at it, and applied.
Thanks.

>      if (!lace_size)
>          return AVERROR(ENOMEM);
>  
> @@ -2836,6 +2836,8 @@ static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
>          uint8_t temp;
>          uint32_t total = 0;
>          for (n = 0; res == 0 && n < *laces - 1; n++) {
> +            lace_size[n] = 0;
> +
>              while (1) {
>                  if (size <= total) {
>                      res = AVERROR_INVALIDDATA;
>
diff mbox

Patch

diff --git a/libavformat/matroskadec.c b/libavformat/matroskadec.c
index 3b8ddc5ecb..4dd933ef74 100644
--- a/libavformat/matroskadec.c
+++ b/libavformat/matroskadec.c
@@ -2814,7 +2814,7 @@  static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
 
     if (!type) {
         *laces    = 1;
-        *lace_buf = av_mallocz(sizeof(int));
+        *lace_buf = av_malloc(sizeof(**lace_buf));
         if (!*lace_buf)
             return AVERROR(ENOMEM);
 
@@ -2826,7 +2826,7 @@  static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
     *laces    = *data + 1;
     data     += 1;
     size     -= 1;
-    lace_size = av_mallocz(*laces * sizeof(int));
+    lace_size = av_malloc(*laces * sizeof(*lace_size));
     if (!lace_size)
         return AVERROR(ENOMEM);
 
@@ -2836,6 +2836,8 @@  static int matroska_parse_laces(MatroskaDemuxContext *matroska, uint8_t **buf,
         uint8_t temp;
         uint32_t total = 0;
         for (n = 0; res == 0 && n < *laces - 1; n++) {
+            lace_size[n] = 0;
+
             while (1) {
                 if (size <= total) {
                     res = AVERROR_INVALIDDATA;