diff mbox series

[FFmpeg-devel,2/2] dnn_backend_native: check operand index

Message ID 1591797657-31866-1-git-send-email-yejun.guo@intel.com
State Accepted
Commit 0b3bd001ac1745d9d008a2d195817df57d7d1d14
Headers show
Series [FFmpeg-devel,1/2] dnn_backend_native.c: refine code for fail case | expand

Checks

Context Check Description
andriy/default pending
andriy/make success Make finished
andriy/make_fate success Make fate finished

Commit Message

Guo, Yejun June 10, 2020, 2 p.m. UTC
it fixed the issue in https://trac.ffmpeg.org/ticket/8716

Signed-off-by: Guo Yejun <yejun.guo@intel.com>
---
 libavfilter/dnn/dnn_backend_native.c                   |  6 +++++-
 libavfilter/dnn/dnn_backend_native_layer_conv2d.c      |  7 ++++++-
 libavfilter/dnn/dnn_backend_native_layer_conv2d.h      |  2 +-
 libavfilter/dnn/dnn_backend_native_layer_depth2space.c |  6 +++++-
 libavfilter/dnn/dnn_backend_native_layer_depth2space.h |  2 +-
 libavfilter/dnn/dnn_backend_native_layer_mathbinary.c  | 12 +++++++++++-
 libavfilter/dnn/dnn_backend_native_layer_mathbinary.h  |  2 +-
 libavfilter/dnn/dnn_backend_native_layer_mathunary.c   |  6 +++++-
 libavfilter/dnn/dnn_backend_native_layer_mathunary.h   |  2 +-
 libavfilter/dnn/dnn_backend_native_layer_maximum.c     |  6 +++++-
 libavfilter/dnn/dnn_backend_native_layer_maximum.h     |  2 +-
 libavfilter/dnn/dnn_backend_native_layer_pad.c         |  6 +++++-
 libavfilter/dnn/dnn_backend_native_layer_pad.h         |  2 +-
 libavfilter/dnn/dnn_backend_native_layers.h            |  2 +-
 14 files changed, 49 insertions(+), 14 deletions(-)

Comments

Guo, Yejun June 16, 2020, 2:33 a.m. UTC | #1
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Guo
> Yejun
> Sent: 2020年6月10日 22:01
> To: ffmpeg-devel@ffmpeg.org
> Cc: Guo, Yejun <yejun.guo@intel.com>
> Subject: [FFmpeg-devel] [PATCH 2/2] dnn_backend_native: check operand index
> 
> it fixed the issue in https://trac.ffmpeg.org/ticket/8716
> 
> Signed-off-by: Guo Yejun <yejun.guo@intel.com>
> ---
>  libavfilter/dnn/dnn_backend_native.c                   |  6 +++++-
>  libavfilter/dnn/dnn_backend_native_layer_conv2d.c      |  7 ++++++-
>  libavfilter/dnn/dnn_backend_native_layer_conv2d.h      |  2 +-
>  libavfilter/dnn/dnn_backend_native_layer_depth2space.c |  6 +++++-
> libavfilter/dnn/dnn_backend_native_layer_depth2space.h |  2 +-
> libavfilter/dnn/dnn_backend_native_layer_mathbinary.c  | 12 +++++++++++-
> libavfilter/dnn/dnn_backend_native_layer_mathbinary.h  |  2 +-
>  libavfilter/dnn/dnn_backend_native_layer_mathunary.c   |  6 +++++-
>  libavfilter/dnn/dnn_backend_native_layer_mathunary.h   |  2 +-
>  libavfilter/dnn/dnn_backend_native_layer_maximum.c     |  6 +++++-
>  libavfilter/dnn/dnn_backend_native_layer_maximum.h     |  2 +-
>  libavfilter/dnn/dnn_backend_native_layer_pad.c         |  6 +++++-
>  libavfilter/dnn/dnn_backend_native_layer_pad.h         |  2 +-
>  libavfilter/dnn/dnn_backend_native_layers.h            |  2 +-
>  14 files changed, 49 insertions(+), 14 deletions(-)
> 
will push tomorrow if no other comments, thanks.
mypopy@gmail.com June 16, 2020, 6:17 a.m. UTC | #2
On Wed, Jun 10, 2020 at 10:04 PM Guo Yejun <yejun.guo@intel.com> wrote:
>
> it fixed the issue in https://trac.ffmpeg.org/ticket/8716
>
> Signed-off-by: Guo Yejun <yejun.guo@intel.com>
> ---
>  libavfilter/dnn/dnn_backend_native.c                   |  6 +++++-
>  libavfilter/dnn/dnn_backend_native_layer_conv2d.c      |  7 ++++++-
>  libavfilter/dnn/dnn_backend_native_layer_conv2d.h      |  2 +-
>  libavfilter/dnn/dnn_backend_native_layer_depth2space.c |  6 +++++-
>  libavfilter/dnn/dnn_backend_native_layer_depth2space.h |  2 +-
>  libavfilter/dnn/dnn_backend_native_layer_mathbinary.c  | 12 +++++++++++-
>  libavfilter/dnn/dnn_backend_native_layer_mathbinary.h  |  2 +-
>  libavfilter/dnn/dnn_backend_native_layer_mathunary.c   |  6 +++++-
>  libavfilter/dnn/dnn_backend_native_layer_mathunary.h   |  2 +-
>  libavfilter/dnn/dnn_backend_native_layer_maximum.c     |  6 +++++-
>  libavfilter/dnn/dnn_backend_native_layer_maximum.h     |  2 +-
>  libavfilter/dnn/dnn_backend_native_layer_pad.c         |  6 +++++-
>  libavfilter/dnn/dnn_backend_native_layer_pad.h         |  2 +-
>  libavfilter/dnn/dnn_backend_native_layers.h            |  2 +-
>  14 files changed, 49 insertions(+), 14 deletions(-)
>
> diff --git a/libavfilter/dnn/dnn_backend_native.c b/libavfilter/dnn/dnn_backend_native.c
> index 12695a0..35236fc 100644
> --- a/libavfilter/dnn/dnn_backend_native.c
> +++ b/libavfilter/dnn/dnn_backend_native.c
> @@ -196,7 +196,7 @@ DNNModel *ff_dnn_load_model_native(const char *model_filename)
>          }
>
>          network->layers[layer].type = layer_type;
> -        parsed_size = layer_funcs[layer_type].pf_load(&network->layers[layer], model_file_context, file_size);
> +        parsed_size = layer_funcs[layer_type].pf_load(&network->layers[layer], model_file_context, file_size, network->operands_num);
>          if (!parsed_size) {
>              goto fail;
>          }
> @@ -209,6 +209,10 @@ DNNModel *ff_dnn_load_model_native(const char *model_filename)
>          int32_t operand_index = (int32_t)avio_rl32(model_file_context);
>          dnn_size += 4;
>
> +        if (operand_index >= network->operands_num) {
> +            goto fail;
> +        }
> +
I prefer
if (expr)
    statements();
without the braces if only a single statement.
>          oprd = &network->operands[operand_index];
>          name_len = (int32_t)avio_rl32(model_file_context);
>          dnn_size += 4;
> diff --git a/libavfilter/dnn/dnn_backend_native_layer_conv2d.c b/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
> index 7b29697..c05bb5e 100644
> --- a/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
> +++ b/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
> @@ -23,7 +23,7 @@
>
>  #define CLAMP_TO_EDGE(x, w) ((x) < 0 ? 0 : ((x) >= (w) ? (w - 1) : (x)))
>
> -int dnn_load_layer_conv2d(Layer *layer, AVIOContext *model_file_context, int file_size)
> +int dnn_load_layer_conv2d(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num)
>  {
>      ConvolutionalParams *conv_params;
>      int kernel_size;
> @@ -80,6 +80,11 @@ int dnn_load_layer_conv2d(Layer *layer, AVIOContext *model_file_context, int fil
>      layer->input_operand_indexes[0] = (int32_t)avio_rl32(model_file_context);
>      layer->output_operand_index = (int32_t)avio_rl32(model_file_context);
>      dnn_size += 8;
> +
> +    if (layer->input_operand_indexes[0] >= operands_num || layer->output_operand_index >= operands_num) {
> +        return 0;
> +    }
> +
>      return dnn_size;
>  }
>
> diff --git a/libavfilter/dnn/dnn_backend_native_layer_conv2d.h b/libavfilter/dnn/dnn_backend_native_layer_conv2d.h
> index bf87264..eeb15fd 100644
> --- a/libavfilter/dnn/dnn_backend_native_layer_conv2d.h
> +++ b/libavfilter/dnn/dnn_backend_native_layer_conv2d.h
> @@ -36,7 +36,7 @@ typedef struct ConvolutionalParams{
>      float *biases;
>  } ConvolutionalParams;
>
> -int dnn_load_layer_conv2d(Layer *layer, AVIOContext *model_file_context, int file_size);
> +int dnn_load_layer_conv2d(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num);
>  int dnn_execute_layer_conv2d(DnnOperand *operands, const int32_t *input_operand_indexes,
>                               int32_t output_operand_index, const void *parameters);
>  #endif
> diff --git a/libavfilter/dnn/dnn_backend_native_layer_depth2space.c b/libavfilter/dnn/dnn_backend_native_layer_depth2space.c
> index 7dab19d..324871c 100644
> --- a/libavfilter/dnn/dnn_backend_native_layer_depth2space.c
> +++ b/libavfilter/dnn/dnn_backend_native_layer_depth2space.c
> @@ -27,7 +27,7 @@
>  #include "libavutil/avassert.h"
>  #include "dnn_backend_native_layer_depth2space.h"
>
> -int dnn_load_layer_depth2space(Layer *layer, AVIOContext *model_file_context, int file_size)
> +int dnn_load_layer_depth2space(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num)
>  {
>      DepthToSpaceParams *params;
>      int dnn_size = 0;
> @@ -42,6 +42,10 @@ int dnn_load_layer_depth2space(Layer *layer, AVIOContext *model_file_context, in
>      dnn_size += 8;
>      layer->params = params;
>
> +    if (layer->input_operand_indexes[0] >= operands_num || layer->output_operand_index >= operands_num) {
> +        return 0;
> +    }
> +
>      return dnn_size;
>  }
>
> diff --git a/libavfilter/dnn/dnn_backend_native_layer_depth2space.h b/libavfilter/dnn/dnn_backend_native_layer_depth2space.h
> index e5465f1..b2901e0 100644
> --- a/libavfilter/dnn/dnn_backend_native_layer_depth2space.h
> +++ b/libavfilter/dnn/dnn_backend_native_layer_depth2space.h
> @@ -34,7 +34,7 @@ typedef struct DepthToSpaceParams{
>      int block_size;
>  } DepthToSpaceParams;
>
> -int dnn_load_layer_depth2space(Layer *layer, AVIOContext *model_file_context, int file_size);
> +int dnn_load_layer_depth2space(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num);
>  int dnn_execute_layer_depth2space(DnnOperand *operands, const int32_t *input_operand_indexes,
>                                    int32_t output_operand_index, const void *parameters);
>
> diff --git a/libavfilter/dnn/dnn_backend_native_layer_mathbinary.c b/libavfilter/dnn/dnn_backend_native_layer_mathbinary.c
> index edc389d..b239a20 100644
> --- a/libavfilter/dnn/dnn_backend_native_layer_mathbinary.c
> +++ b/libavfilter/dnn/dnn_backend_native_layer_mathbinary.c
> @@ -27,7 +27,7 @@
>  #include "libavutil/avassert.h"
>  #include "dnn_backend_native_layer_mathbinary.h"
>
> -int dnn_load_layer_math_binary(Layer *layer, AVIOContext *model_file_context, int file_size)
> +int dnn_load_layer_math_binary(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num)
>  {
>      DnnLayerMathBinaryParams *params;
>      int dnn_size = 0;
> @@ -45,6 +45,9 @@ int dnn_load_layer_math_binary(Layer *layer, AVIOContext *model_file_context, in
>          params->v = av_int2float(avio_rl32(model_file_context));
>      } else {
>          layer->input_operand_indexes[input_index] = (int32_t)avio_rl32(model_file_context);
> +        if (layer->input_operand_indexes[input_index] >= operands_num) {
> +            return 0;
> +        }
>          input_index++;
>      }
>      dnn_size += 4;
> @@ -55,6 +58,9 @@ int dnn_load_layer_math_binary(Layer *layer, AVIOContext *model_file_context, in
>          params->v = av_int2float(avio_rl32(model_file_context));
>      } else {
>          layer->input_operand_indexes[input_index] = (int32_t)avio_rl32(model_file_context);
> +        if (layer->input_operand_indexes[input_index] >= operands_num) {
> +            return 0;
> +        }
>          input_index++;
>      }
>      dnn_size += 4;
> @@ -63,6 +69,10 @@ int dnn_load_layer_math_binary(Layer *layer, AVIOContext *model_file_context, in
>      dnn_size += 4;
>      layer->params = params;
>
> +    if (layer->output_operand_index >= operands_num) {
> +        return 0;
> +    }
> +
>      return dnn_size;
>  }
>
> diff --git a/libavfilter/dnn/dnn_backend_native_layer_mathbinary.h b/libavfilter/dnn/dnn_backend_native_layer_mathbinary.h
> index f3dbbeb..0acf3b0 100644
> --- a/libavfilter/dnn/dnn_backend_native_layer_mathbinary.h
> +++ b/libavfilter/dnn/dnn_backend_native_layer_mathbinary.h
> @@ -46,7 +46,7 @@ typedef struct DnnLayerMathBinaryParams{
>      float v;
>  } DnnLayerMathBinaryParams;
>
> -int dnn_load_layer_math_binary(Layer *layer, AVIOContext *model_file_context, int file_size);
> +int dnn_load_layer_math_binary(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num);
>  int dnn_execute_layer_math_binary(DnnOperand *operands, const int32_t *input_operand_indexes,
>                                   int32_t output_operand_index, const void *parameters);
>
> diff --git a/libavfilter/dnn/dnn_backend_native_layer_mathunary.c b/libavfilter/dnn/dnn_backend_native_layer_mathunary.c
> index d65af15..0d3627f 100644
> --- a/libavfilter/dnn/dnn_backend_native_layer_mathunary.c
> +++ b/libavfilter/dnn/dnn_backend_native_layer_mathunary.c
> @@ -27,7 +27,7 @@
>  #include "libavutil/avassert.h"
>  #include "dnn_backend_native_layer_mathunary.h"
>
> -int dnn_load_layer_math_unary(Layer *layer, AVIOContext *model_file_context, int file_size)
> +int dnn_load_layer_math_unary(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num)
>  {
>      DnnLayerMathUnaryParams *params;
>      int dnn_size = 0;
> @@ -42,6 +42,10 @@ int dnn_load_layer_math_unary(Layer *layer, AVIOContext *model_file_context, int
>      layer->output_operand_index = (int32_t)avio_rl32(model_file_context);
>      dnn_size += 8;
>
> +    if (layer->input_operand_indexes[0] >= operands_num || layer->output_operand_index >= operands_num) {
> +        return 0;
> +    }
> +
>      return dnn_size;
>
>  }
> diff --git a/libavfilter/dnn/dnn_backend_native_layer_mathunary.h b/libavfilter/dnn/dnn_backend_native_layer_mathunary.h
> index 4e44003..a9a8a0d 100644
> --- a/libavfilter/dnn/dnn_backend_native_layer_mathunary.h
> +++ b/libavfilter/dnn/dnn_backend_native_layer_mathunary.h
> @@ -38,7 +38,7 @@ typedef struct DnnLayerMathUnaryParams{
>      DNNMathUnaryOperation un_op;
>  } DnnLayerMathUnaryParams;
>
> -int dnn_load_layer_math_unary(Layer *layer, AVIOContext *model_file_context, int file_size);
> +int dnn_load_layer_math_unary(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num);
>  int dnn_execute_layer_math_unary(DnnOperand *operands, const int32_t *input_operand_indexes,
>                                  int32_t output_operand_index, const void *parameters);
>
> diff --git a/libavfilter/dnn/dnn_backend_native_layer_maximum.c b/libavfilter/dnn/dnn_backend_native_layer_maximum.c
> index 19f0e8d..af16e08 100644
> --- a/libavfilter/dnn/dnn_backend_native_layer_maximum.c
> +++ b/libavfilter/dnn/dnn_backend_native_layer_maximum.c
> @@ -27,7 +27,7 @@
>  #include "libavutil/avassert.h"
>  #include "dnn_backend_native_layer_maximum.h"
>
> -int dnn_load_layer_maximum(Layer *layer, AVIOContext *model_file_context, int file_size)
> +int dnn_load_layer_maximum(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num)
>  {
>      DnnLayerMaximumParams *params;
>      int dnn_size = 0;
> @@ -42,6 +42,10 @@ int dnn_load_layer_maximum(Layer *layer, AVIOContext *model_file_context, int fi
>      layer->output_operand_index = (int32_t)avio_rl32(model_file_context);
>      dnn_size += 8;
>
> +    if (layer->input_operand_indexes[0] >= operands_num || layer->output_operand_index >= operands_num) {
> +        return 0;
> +    }
> +
>      return dnn_size;
>  }
>
> diff --git a/libavfilter/dnn/dnn_backend_native_layer_maximum.h b/libavfilter/dnn/dnn_backend_native_layer_maximum.h
> index 601158b..c049c63 100644
> --- a/libavfilter/dnn/dnn_backend_native_layer_maximum.h
> +++ b/libavfilter/dnn/dnn_backend_native_layer_maximum.h
> @@ -37,7 +37,7 @@ typedef struct DnnLayerMaximumParams{
>      }val;
>  } DnnLayerMaximumParams;
>
> -int dnn_load_layer_maximum(Layer *layer, AVIOContext *model_file_context, int file_size);
> +int dnn_load_layer_maximum(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num);
>  int dnn_execute_layer_maximum(DnnOperand *operands, const int32_t *input_operand_indexes,
>                                int32_t output_operand_index, const void *parameters);
>
> diff --git a/libavfilter/dnn/dnn_backend_native_layer_pad.c b/libavfilter/dnn/dnn_backend_native_layer_pad.c
> index 8e5959b..dfbd204 100644
> --- a/libavfilter/dnn/dnn_backend_native_layer_pad.c
> +++ b/libavfilter/dnn/dnn_backend_native_layer_pad.c
> @@ -22,7 +22,7 @@
>  #include "libavutil/avassert.h"
>  #include "dnn_backend_native_layer_pad.h"
>
> -int dnn_load_layer_pad(Layer *layer, AVIOContext *model_file_context, int file_size)
> +int dnn_load_layer_pad(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num)
>  {
>      LayerPadParams *params;
>      int dnn_size = 0;
> @@ -42,6 +42,10 @@ int dnn_load_layer_pad(Layer *layer, AVIOContext *model_file_context, int file_s
>      dnn_size += 8;
>      layer->params = params;
>
> +    if (layer->input_operand_indexes[0] >= operands_num || layer->output_operand_index >= operands_num) {
> +        return 0;
> +    }
> +
>      return dnn_size;
>  }
>
> diff --git a/libavfilter/dnn/dnn_backend_native_layer_pad.h b/libavfilter/dnn/dnn_backend_native_layer_pad.h
> index 936a9bd..18e05bd 100644
> --- a/libavfilter/dnn/dnn_backend_native_layer_pad.h
> +++ b/libavfilter/dnn/dnn_backend_native_layer_pad.h
> @@ -36,7 +36,7 @@ typedef struct LayerPadParams{
>      float constant_values;
>  } LayerPadParams;
>
> -int dnn_load_layer_pad(Layer *layer, AVIOContext *model_file_context, int file_size);
> +int dnn_load_layer_pad(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num);
>  int dnn_execute_layer_pad(DnnOperand *operands, const int32_t *input_operand_indexes,
>                            int32_t output_operand_index, const void *parameters);
>
> diff --git a/libavfilter/dnn/dnn_backend_native_layers.h b/libavfilter/dnn/dnn_backend_native_layers.h
> index 2df0ce9..b696e9c 100644
> --- a/libavfilter/dnn/dnn_backend_native_layers.h
> +++ b/libavfilter/dnn/dnn_backend_native_layers.h
> @@ -26,7 +26,7 @@
>
>  typedef int (*LAYER_EXEC_FUNC)(DnnOperand *operands, const int32_t *input_operand_indexes,
>                                 int32_t output_operand_index, const void *parameters);
> -typedef int (*LAYER_LOAD_FUNC)(Layer *layer, AVIOContext *model_file_context, int file_size);
> +typedef int (*LAYER_LOAD_FUNC)(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num);
>
>  typedef struct LayerFunc {
>      LAYER_EXEC_FUNC pf_exec;
> --
> 2.7.4
>
> _______________________________________________
> 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".
Guo, Yejun June 16, 2020, 8:18 a.m. UTC | #3
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> mypopy@gmail.com
> Sent: 2020年6月16日 14:18
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 2/2] dnn_backend_native: check operand
> index
> 
> On Wed, Jun 10, 2020 at 10:04 PM Guo Yejun <yejun.guo@intel.com> wrote:
> >
> > it fixed the issue in https://trac.ffmpeg.org/ticket/8716
> >
> > Signed-off-by: Guo Yejun <yejun.guo@intel.com>
> >
> > +        if (operand_index >= network->operands_num) {
> > +            goto fail;
> > +        }
> > +
> I prefer
> if (expr)
>     statements();
> without the braces if only a single statement.

thanks for the review. 

I personally like to always add '{}' to avoid possible mistakes when we change code in the future. Some coding styles also recommend this method.

I checked at http://ffmpeg.org/developer.html#Coding-Rules and do not find an explicit description. But I just remembered that someone also mentioned this some time ago. I'll change to this style in v2 if no support from others for my style.
Carl Eugen Hoyos June 16, 2020, 10:57 a.m. UTC | #4
> Am 16.06.2020 um 10:18 schrieb Guo, Yejun <yejun.guo@intel.com>:
> 
> I personally like to always add '{}' to avoid possible mistakes when we change code in the future. Some coding styles also recommend this method.
> 
> I checked at http://ffmpeg.org/developer.html#Coding-Rules and do not find an explicit description. But I just remembered that someone also mentioned this some time ago. I'll change to this style in v2 if no support from others for my style.

I support your style but it is only the file maintainer‘s decision.

Carl Eugen
Guo, Yejun June 17, 2020, 3:23 a.m. UTC | #5
> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Carl
> Eugen Hoyos
> Sent: 2020年6月16日 18:57
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH 2/2] dnn_backend_native: check operand
> index
> 
> 
> 
> > Am 16.06.2020 um 10:18 schrieb Guo, Yejun <yejun.guo@intel.com>:
> >
> > I personally like to always add '{}' to avoid possible mistakes when we change
> code in the future. Some coding styles also recommend this method.
> >
> > I checked at http://ffmpeg.org/developer.html#Coding-Rules and do not find
> an explicit description. But I just remembered that someone also mentioned this
> some time ago. I'll change to this style in v2 if no support from others for my
> style.
> 
> I support your style but it is only the file maintainer‘s decision.

thanks, I'll keep the V1 patch and push soon.

> 
> Carl Eugen
> _______________________________________________
> 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".
Michael Niedermayer July 1, 2020, 11:45 p.m. UTC | #6
On Wed, Jun 17, 2020 at 03:23:34AM +0000, Guo, Yejun wrote:
> 
> 
> > -----Original Message-----
> > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Carl
> > Eugen Hoyos
> > Sent: 2020年6月16日 18:57
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH 2/2] dnn_backend_native: check operand
> > index
> > 
> > 
> > 
> > > Am 16.06.2020 um 10:18 schrieb Guo, Yejun <yejun.guo@intel.com>:
> > >
> > > I personally like to always add '{}' to avoid possible mistakes when we change
> > code in the future. Some coding styles also recommend this method.
> > >
> > > I checked at http://ffmpeg.org/developer.html#Coding-Rules and do not find
> > an explicit description. But I just remembered that someone also mentioned this
> > some time ago. I'll change to this style in v2 if no support from others for my
> > style.
> > 
> > I support your style but it is only the file maintainer‘s decision.
> 
> thanks, I'll keep the V1 patch and push soon.

please also apply this to affected release branches


[...]
Guo, Yejun July 2, 2020, 1:30 a.m. UTC | #7
> -----Original Message-----
> From: Michael Niedermayer <michael@niedermayer.cc>
> Sent: 2020年7月2日 7:45
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>;
> Guo, Yejun <yejun.guo@intel.com>
> Subject: Re: [FFmpeg-devel] [PATCH 2/2] dnn_backend_native: check operand
> index
> 
> On Wed, Jun 17, 2020 at 03:23:34AM +0000, Guo, Yejun wrote:
> >
> >
> > > -----Original Message-----
> > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > Carl Eugen Hoyos
> > > Sent: 2020年6月16日 18:57
> > > To: FFmpeg development discussions and patches
> > > <ffmpeg-devel@ffmpeg.org>
> > > Subject: Re: [FFmpeg-devel] [PATCH 2/2] dnn_backend_native: check
> > > operand index
> > >
> > >
> > >
> > > > Am 16.06.2020 um 10:18 schrieb Guo, Yejun <yejun.guo@intel.com>:
> > > >
> > > > I personally like to always add '{}' to avoid possible mistakes
> > > > when we change
> > > code in the future. Some coding styles also recommend this method.
> > > >
> > > > I checked at http://ffmpeg.org/developer.html#Coding-Rules and do
> > > > not find
> > > an explicit description. But I just remembered that someone also
> > > mentioned this some time ago. I'll change to this style in v2 if no
> > > support from others for my style.
> > >
> > > I support your style but it is only the file maintainer‘s decision.
> >
> > thanks, I'll keep the V1 patch and push soon.
> 
> please also apply this to affected release branches

done, pushed to branch 4.3 with dd273d359e45ab69398ac0dc41206d5f1a9371bf and 5530748bfdf1a4d41d4c92e59f662c94e38a5f94,
no need to apply 4.2 and older.
Michael Niedermayer July 2, 2020, 8:34 a.m. UTC | #8
On Thu, Jul 02, 2020 at 01:30:34AM +0000, Guo, Yejun wrote:
> 
> 
> > -----Original Message-----
> > From: Michael Niedermayer <michael@niedermayer.cc>
> > Sent: 2020年7月2日 7:45
> > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>;
> > Guo, Yejun <yejun.guo@intel.com>
> > Subject: Re: [FFmpeg-devel] [PATCH 2/2] dnn_backend_native: check operand
> > index
> > 
> > On Wed, Jun 17, 2020 at 03:23:34AM +0000, Guo, Yejun wrote:
> > >
> > >
> > > > -----Original Message-----
> > > > From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> > > > Carl Eugen Hoyos
> > > > Sent: 2020年6月16日 18:57
> > > > To: FFmpeg development discussions and patches
> > > > <ffmpeg-devel@ffmpeg.org>
> > > > Subject: Re: [FFmpeg-devel] [PATCH 2/2] dnn_backend_native: check
> > > > operand index
> > > >
> > > >
> > > >
> > > > > Am 16.06.2020 um 10:18 schrieb Guo, Yejun <yejun.guo@intel.com>:
> > > > >
> > > > > I personally like to always add '{}' to avoid possible mistakes
> > > > > when we change
> > > > code in the future. Some coding styles also recommend this method.
> > > > >
> > > > > I checked at http://ffmpeg.org/developer.html#Coding-Rules and do
> > > > > not find
> > > > an explicit description. But I just remembered that someone also
> > > > mentioned this some time ago. I'll change to this style in v2 if no
> > > > support from others for my style.
> > > >
> > > > I support your style but it is only the file maintainer‘s decision.
> > >
> > > thanks, I'll keep the V1 patch and push soon.
> > 
> > please also apply this to affected release branches
> 
> done, pushed to branch 4.3 with dd273d359e45ab69398ac0dc41206d5f1a9371bf and 5530748bfdf1a4d41d4c92e59f662c94e38a5f94,
> no need to apply 4.2 and older.

thanks

[...]
diff mbox series

Patch

diff --git a/libavfilter/dnn/dnn_backend_native.c b/libavfilter/dnn/dnn_backend_native.c
index 12695a0..35236fc 100644
--- a/libavfilter/dnn/dnn_backend_native.c
+++ b/libavfilter/dnn/dnn_backend_native.c
@@ -196,7 +196,7 @@  DNNModel *ff_dnn_load_model_native(const char *model_filename)
         }
 
         network->layers[layer].type = layer_type;
-        parsed_size = layer_funcs[layer_type].pf_load(&network->layers[layer], model_file_context, file_size);
+        parsed_size = layer_funcs[layer_type].pf_load(&network->layers[layer], model_file_context, file_size, network->operands_num);
         if (!parsed_size) {
             goto fail;
         }
@@ -209,6 +209,10 @@  DNNModel *ff_dnn_load_model_native(const char *model_filename)
         int32_t operand_index = (int32_t)avio_rl32(model_file_context);
         dnn_size += 4;
 
+        if (operand_index >= network->operands_num) {
+            goto fail;
+        }
+
         oprd = &network->operands[operand_index];
         name_len = (int32_t)avio_rl32(model_file_context);
         dnn_size += 4;
diff --git a/libavfilter/dnn/dnn_backend_native_layer_conv2d.c b/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
index 7b29697..c05bb5e 100644
--- a/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
+++ b/libavfilter/dnn/dnn_backend_native_layer_conv2d.c
@@ -23,7 +23,7 @@ 
 
 #define CLAMP_TO_EDGE(x, w) ((x) < 0 ? 0 : ((x) >= (w) ? (w - 1) : (x)))
 
-int dnn_load_layer_conv2d(Layer *layer, AVIOContext *model_file_context, int file_size)
+int dnn_load_layer_conv2d(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num)
 {
     ConvolutionalParams *conv_params;
     int kernel_size;
@@ -80,6 +80,11 @@  int dnn_load_layer_conv2d(Layer *layer, AVIOContext *model_file_context, int fil
     layer->input_operand_indexes[0] = (int32_t)avio_rl32(model_file_context);
     layer->output_operand_index = (int32_t)avio_rl32(model_file_context);
     dnn_size += 8;
+
+    if (layer->input_operand_indexes[0] >= operands_num || layer->output_operand_index >= operands_num) {
+        return 0;
+    }
+
     return dnn_size;
 }
 
diff --git a/libavfilter/dnn/dnn_backend_native_layer_conv2d.h b/libavfilter/dnn/dnn_backend_native_layer_conv2d.h
index bf87264..eeb15fd 100644
--- a/libavfilter/dnn/dnn_backend_native_layer_conv2d.h
+++ b/libavfilter/dnn/dnn_backend_native_layer_conv2d.h
@@ -36,7 +36,7 @@  typedef struct ConvolutionalParams{
     float *biases;
 } ConvolutionalParams;
 
-int dnn_load_layer_conv2d(Layer *layer, AVIOContext *model_file_context, int file_size);
+int dnn_load_layer_conv2d(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num);
 int dnn_execute_layer_conv2d(DnnOperand *operands, const int32_t *input_operand_indexes,
                              int32_t output_operand_index, const void *parameters);
 #endif
diff --git a/libavfilter/dnn/dnn_backend_native_layer_depth2space.c b/libavfilter/dnn/dnn_backend_native_layer_depth2space.c
index 7dab19d..324871c 100644
--- a/libavfilter/dnn/dnn_backend_native_layer_depth2space.c
+++ b/libavfilter/dnn/dnn_backend_native_layer_depth2space.c
@@ -27,7 +27,7 @@ 
 #include "libavutil/avassert.h"
 #include "dnn_backend_native_layer_depth2space.h"
 
-int dnn_load_layer_depth2space(Layer *layer, AVIOContext *model_file_context, int file_size)
+int dnn_load_layer_depth2space(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num)
 {
     DepthToSpaceParams *params;
     int dnn_size = 0;
@@ -42,6 +42,10 @@  int dnn_load_layer_depth2space(Layer *layer, AVIOContext *model_file_context, in
     dnn_size += 8;
     layer->params = params;
 
+    if (layer->input_operand_indexes[0] >= operands_num || layer->output_operand_index >= operands_num) {
+        return 0;
+    }
+
     return dnn_size;
 }
 
diff --git a/libavfilter/dnn/dnn_backend_native_layer_depth2space.h b/libavfilter/dnn/dnn_backend_native_layer_depth2space.h
index e5465f1..b2901e0 100644
--- a/libavfilter/dnn/dnn_backend_native_layer_depth2space.h
+++ b/libavfilter/dnn/dnn_backend_native_layer_depth2space.h
@@ -34,7 +34,7 @@  typedef struct DepthToSpaceParams{
     int block_size;
 } DepthToSpaceParams;
 
-int dnn_load_layer_depth2space(Layer *layer, AVIOContext *model_file_context, int file_size);
+int dnn_load_layer_depth2space(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num);
 int dnn_execute_layer_depth2space(DnnOperand *operands, const int32_t *input_operand_indexes,
                                   int32_t output_operand_index, const void *parameters);
 
diff --git a/libavfilter/dnn/dnn_backend_native_layer_mathbinary.c b/libavfilter/dnn/dnn_backend_native_layer_mathbinary.c
index edc389d..b239a20 100644
--- a/libavfilter/dnn/dnn_backend_native_layer_mathbinary.c
+++ b/libavfilter/dnn/dnn_backend_native_layer_mathbinary.c
@@ -27,7 +27,7 @@ 
 #include "libavutil/avassert.h"
 #include "dnn_backend_native_layer_mathbinary.h"
 
-int dnn_load_layer_math_binary(Layer *layer, AVIOContext *model_file_context, int file_size)
+int dnn_load_layer_math_binary(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num)
 {
     DnnLayerMathBinaryParams *params;
     int dnn_size = 0;
@@ -45,6 +45,9 @@  int dnn_load_layer_math_binary(Layer *layer, AVIOContext *model_file_context, in
         params->v = av_int2float(avio_rl32(model_file_context));
     } else {
         layer->input_operand_indexes[input_index] = (int32_t)avio_rl32(model_file_context);
+        if (layer->input_operand_indexes[input_index] >= operands_num) {
+            return 0;
+        }
         input_index++;
     }
     dnn_size += 4;
@@ -55,6 +58,9 @@  int dnn_load_layer_math_binary(Layer *layer, AVIOContext *model_file_context, in
         params->v = av_int2float(avio_rl32(model_file_context));
     } else {
         layer->input_operand_indexes[input_index] = (int32_t)avio_rl32(model_file_context);
+        if (layer->input_operand_indexes[input_index] >= operands_num) {
+            return 0;
+        }
         input_index++;
     }
     dnn_size += 4;
@@ -63,6 +69,10 @@  int dnn_load_layer_math_binary(Layer *layer, AVIOContext *model_file_context, in
     dnn_size += 4;
     layer->params = params;
 
+    if (layer->output_operand_index >= operands_num) {
+        return 0;
+    }
+
     return dnn_size;
 }
 
diff --git a/libavfilter/dnn/dnn_backend_native_layer_mathbinary.h b/libavfilter/dnn/dnn_backend_native_layer_mathbinary.h
index f3dbbeb..0acf3b0 100644
--- a/libavfilter/dnn/dnn_backend_native_layer_mathbinary.h
+++ b/libavfilter/dnn/dnn_backend_native_layer_mathbinary.h
@@ -46,7 +46,7 @@  typedef struct DnnLayerMathBinaryParams{
     float v;
 } DnnLayerMathBinaryParams;
 
-int dnn_load_layer_math_binary(Layer *layer, AVIOContext *model_file_context, int file_size);
+int dnn_load_layer_math_binary(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num);
 int dnn_execute_layer_math_binary(DnnOperand *operands, const int32_t *input_operand_indexes,
                                  int32_t output_operand_index, const void *parameters);
 
diff --git a/libavfilter/dnn/dnn_backend_native_layer_mathunary.c b/libavfilter/dnn/dnn_backend_native_layer_mathunary.c
index d65af15..0d3627f 100644
--- a/libavfilter/dnn/dnn_backend_native_layer_mathunary.c
+++ b/libavfilter/dnn/dnn_backend_native_layer_mathunary.c
@@ -27,7 +27,7 @@ 
 #include "libavutil/avassert.h"
 #include "dnn_backend_native_layer_mathunary.h"
 
-int dnn_load_layer_math_unary(Layer *layer, AVIOContext *model_file_context, int file_size)
+int dnn_load_layer_math_unary(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num)
 {
     DnnLayerMathUnaryParams *params;
     int dnn_size = 0;
@@ -42,6 +42,10 @@  int dnn_load_layer_math_unary(Layer *layer, AVIOContext *model_file_context, int
     layer->output_operand_index = (int32_t)avio_rl32(model_file_context);
     dnn_size += 8;
 
+    if (layer->input_operand_indexes[0] >= operands_num || layer->output_operand_index >= operands_num) {
+        return 0;
+    }
+
     return dnn_size;
 
 }
diff --git a/libavfilter/dnn/dnn_backend_native_layer_mathunary.h b/libavfilter/dnn/dnn_backend_native_layer_mathunary.h
index 4e44003..a9a8a0d 100644
--- a/libavfilter/dnn/dnn_backend_native_layer_mathunary.h
+++ b/libavfilter/dnn/dnn_backend_native_layer_mathunary.h
@@ -38,7 +38,7 @@  typedef struct DnnLayerMathUnaryParams{
     DNNMathUnaryOperation un_op;
 } DnnLayerMathUnaryParams;
 
-int dnn_load_layer_math_unary(Layer *layer, AVIOContext *model_file_context, int file_size);
+int dnn_load_layer_math_unary(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num);
 int dnn_execute_layer_math_unary(DnnOperand *operands, const int32_t *input_operand_indexes,
                                 int32_t output_operand_index, const void *parameters);
 
diff --git a/libavfilter/dnn/dnn_backend_native_layer_maximum.c b/libavfilter/dnn/dnn_backend_native_layer_maximum.c
index 19f0e8d..af16e08 100644
--- a/libavfilter/dnn/dnn_backend_native_layer_maximum.c
+++ b/libavfilter/dnn/dnn_backend_native_layer_maximum.c
@@ -27,7 +27,7 @@ 
 #include "libavutil/avassert.h"
 #include "dnn_backend_native_layer_maximum.h"
 
-int dnn_load_layer_maximum(Layer *layer, AVIOContext *model_file_context, int file_size)
+int dnn_load_layer_maximum(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num)
 {
     DnnLayerMaximumParams *params;
     int dnn_size = 0;
@@ -42,6 +42,10 @@  int dnn_load_layer_maximum(Layer *layer, AVIOContext *model_file_context, int fi
     layer->output_operand_index = (int32_t)avio_rl32(model_file_context);
     dnn_size += 8;
 
+    if (layer->input_operand_indexes[0] >= operands_num || layer->output_operand_index >= operands_num) {
+        return 0;
+    }
+
     return dnn_size;
 }
 
diff --git a/libavfilter/dnn/dnn_backend_native_layer_maximum.h b/libavfilter/dnn/dnn_backend_native_layer_maximum.h
index 601158b..c049c63 100644
--- a/libavfilter/dnn/dnn_backend_native_layer_maximum.h
+++ b/libavfilter/dnn/dnn_backend_native_layer_maximum.h
@@ -37,7 +37,7 @@  typedef struct DnnLayerMaximumParams{
     }val;
 } DnnLayerMaximumParams;
 
-int dnn_load_layer_maximum(Layer *layer, AVIOContext *model_file_context, int file_size);
+int dnn_load_layer_maximum(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num);
 int dnn_execute_layer_maximum(DnnOperand *operands, const int32_t *input_operand_indexes,
                               int32_t output_operand_index, const void *parameters);
 
diff --git a/libavfilter/dnn/dnn_backend_native_layer_pad.c b/libavfilter/dnn/dnn_backend_native_layer_pad.c
index 8e5959b..dfbd204 100644
--- a/libavfilter/dnn/dnn_backend_native_layer_pad.c
+++ b/libavfilter/dnn/dnn_backend_native_layer_pad.c
@@ -22,7 +22,7 @@ 
 #include "libavutil/avassert.h"
 #include "dnn_backend_native_layer_pad.h"
 
-int dnn_load_layer_pad(Layer *layer, AVIOContext *model_file_context, int file_size)
+int dnn_load_layer_pad(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num)
 {
     LayerPadParams *params;
     int dnn_size = 0;
@@ -42,6 +42,10 @@  int dnn_load_layer_pad(Layer *layer, AVIOContext *model_file_context, int file_s
     dnn_size += 8;
     layer->params = params;
 
+    if (layer->input_operand_indexes[0] >= operands_num || layer->output_operand_index >= operands_num) {
+        return 0;
+    }
+
     return dnn_size;
 }
 
diff --git a/libavfilter/dnn/dnn_backend_native_layer_pad.h b/libavfilter/dnn/dnn_backend_native_layer_pad.h
index 936a9bd..18e05bd 100644
--- a/libavfilter/dnn/dnn_backend_native_layer_pad.h
+++ b/libavfilter/dnn/dnn_backend_native_layer_pad.h
@@ -36,7 +36,7 @@  typedef struct LayerPadParams{
     float constant_values;
 } LayerPadParams;
 
-int dnn_load_layer_pad(Layer *layer, AVIOContext *model_file_context, int file_size);
+int dnn_load_layer_pad(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num);
 int dnn_execute_layer_pad(DnnOperand *operands, const int32_t *input_operand_indexes,
                           int32_t output_operand_index, const void *parameters);
 
diff --git a/libavfilter/dnn/dnn_backend_native_layers.h b/libavfilter/dnn/dnn_backend_native_layers.h
index 2df0ce9..b696e9c 100644
--- a/libavfilter/dnn/dnn_backend_native_layers.h
+++ b/libavfilter/dnn/dnn_backend_native_layers.h
@@ -26,7 +26,7 @@ 
 
 typedef int (*LAYER_EXEC_FUNC)(DnnOperand *operands, const int32_t *input_operand_indexes,
                                int32_t output_operand_index, const void *parameters);
-typedef int (*LAYER_LOAD_FUNC)(Layer *layer, AVIOContext *model_file_context, int file_size);
+typedef int (*LAYER_LOAD_FUNC)(Layer *layer, AVIOContext *model_file_context, int file_size, int operands_num);
 
 typedef struct LayerFunc {
     LAYER_EXEC_FUNC pf_exec;