From patchwork Mon Jul 6 07:32:17 2020 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Patchwork-Submitter: =?utf-8?q?Reimar_D=C3=B6ffinger?= X-Patchwork-Id: 20839 Return-Path: X-Original-To: patchwork@ffaux-bg.ffmpeg.org Delivered-To: patchwork@ffaux-bg.ffmpeg.org Received: from ffbox0-bg.mplayerhq.hu (ffbox0-bg.ffmpeg.org [79.124.17.100]) by ffaux.localdomain (Postfix) with ESMTP id C616B44A55B for ; Mon, 6 Jul 2020 10:32:24 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id A4E6368B0EE; Mon, 6 Jul 2020 10:32:24 +0300 (EEST) X-Original-To: ffmpeg-devel@ffmpeg.org Delivered-To: ffmpeg-devel@ffmpeg.org Received: from mout-xforward.gmx.net (mout-xforward.gmx.net [82.165.159.40]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 78BD3689F05 for ; Mon, 6 Jul 2020 10:32:18 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1594020738; bh=sj6KFXBwZeD3MMMJ9OgQZuAkRJs8trlAlqEPhD6g2yo=; h=X-UI-Sender-Class:From:To:Subject:Date:In-Reply-To:References; b=GXdg4/Q2cULZY1bFva5lvG+yHRfPILkYHhcnDzZu/lWykIKh0SGGZ1JQZnu5CLOi+ xWnUC/5UTjjyIf73DPNLaILx3oMz4+biCPh0yoszqluRU82+v5xnmhZqVU5Zyc4kzB UQ2V4y3SoYXWJJ/2HiuOVdFrO1fYBz8Mrr+q8A00= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from localhost ([81.170.149.220]) by mail.gmx.com (mrgmx105 [212.227.17.168]) with ESMTPSA (Nemesis) id 1MbRfl-1kTtDk3KDh-00bw0A for ; Mon, 06 Jul 2020 09:32:17 +0200 From: =?utf-8?q?Reimar_D=C3=B6ffinger?= To: ffmpeg-devel@ffmpeg.org Date: Mon, 6 Jul 2020 09:32:17 +0200 Message-Id: <20200706073217.3104-1-Reimar.Doeffinger@gmx.de> X-Mailer: git-send-email 2.27.0 In-Reply-To: <20200706065925.4814-1-Reimar.Doeffinger@gmx.de> References: <20200706065925.4814-1-Reimar.Doeffinger@gmx.de> MIME-Version: 1.0 X-Provags-ID: V03:K1:Wg2pK4QFcIx597xPMorLUiQEhcBrMDM23Nn4sb8y1HrBx4MQRcq iblvgPS9tcX9kC2IPQwvJI7WesGM0T/wt9UwqnSlOZXxL6oRgap/TDYIqkpGwEj5RoQCq3g l3dtXpUnmODEgSzuNoxuCbQA4B2HLjq7vutYpazd6Rc0jtwQkvHcnNzAjlYZPKM+74ijcvx mwdsXxgIZ9/YAYuida89g== X-Spam-Flag: YES X-UI-Out-Filterresults: junk:10;V03:K0:Hz57dR+8EsY=:sOy5OksDp3dAXSxV8IafvIq5 Y5pvmsSJPFMwajLGyOcc0wirz7Enov4/9bB/Sv1qfcfku5csf5cL5lVn0lR6E+kIjsIWWS7Eh 748wEiHM0HxwY7lyFQCd3TrRldVs5jUgnYyKpMsKTttJFCDzkSdlbCUCVf+1T3O/35K5MIJ56 idBwrlxfqGRryNLW5cUIM6aEv047/7H+e8ImVWMVAxqmiw2+qqznH+oZE8V3Ip2QA1bPYhbVw dCZblzC3+zABdrTg5KdI/UWOvH+fHpIqZyJ+jST7X0oLb0xdSv5hBrPrzWOdquZYMfgHBZKQk j6tnu65gupM2IICI7zVXmhX89i1twV6jwMXUs0KzAo3DxcjPHbq+xvEAKuiALrcIKlxHbU1rK VYyBvrFEb96oVJYRuarMnGyBisYVL4auGORz0OQ7B5g1UKcbxcoEwDeByi2YdwLqRsvttKQrP 91GWA5pEFKI+x2z/ygiaA6p8qnlD7SkGdgSAyn0SiqdVgf2x17PvCKs4M5ERPUsGi5iHgKH14 A2mWmjgW3Tqn4XMLyydS7DaihoVhCgv25H1srvoGdwiu58oWToH6YlXSEO8J/ANLGAn0PMIyK Ydt5Ci1A/NIzhZT/6S/746BozUctKK7GsFmEkWIN8xcF/j16N8lbzYAtSiclE9oV/otZoQWxZ NIIeNCAGUxAe+lLDIcmAbCITtUSO1IWWCpTzVBEtTx83zT8ADIqljk8jHvcO+sib+GPPaZUEu 4uDsn43ftjSdg4xU3e3rrmVDGWdCzBn8I8PtwLoV5rGPDLhlHyUng5H+Rxe54Qc2KfgU5YRiG HvBdB/bpS45ibCXtdWWyuaIEGDKUbMglrM7BEh6axAllklSHqe3jJupsorgv0/n+7Yg/+jiii PoUZJqoOldxT2oEwCbDANERIO0L4qBNhPb1jvfQUbyi4I59IFXtWzxU9xwt4LfZEPhUSc3nP0 yiqFS6MFvMbC2kC+GJFO4Zt9DN0sFgQdEoYavMlvR3o+9Z+hTf67hJ7kpM2vJ6UFjdCA6UoOW GY0jLApHrrsa6K+5FvSiZz25mGUqtzd+vXpkWFwFU4im+983hKIn31Kf40fNK2fmaAdYAJXkZ 553ycAq12xiUEtrBfTRXY4ZWzIwGSwmyvAPd/I3DFfza+bgtFxDBVzUrWa7yfyEFa2yGzNm2P iwLaah2edw4ncfw1bNeOeX0kV6iiSKPb+kxZFN/zaK4w6AvmpGJaMa+j6DGYBYTLUg1ZpebA2 n9X5L+vg9Lv8tyr2NgOsDkSdj/xIKaPfM47kg== Subject: [FFmpeg-devel] [PATCH] dnn_backend_native: Add overflow check for length calculation. X-BeenThere: ffmpeg-devel@ffmpeg.org X-Mailman-Version: 2.1.20 Precedence: list List-Id: FFmpeg development discussions and patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: FFmpeg development discussions and patches Errors-To: ffmpeg-devel-bounces@ffmpeg.org Sender: "ffmpeg-devel" We should not silently allocate an incorrect sized buffer. Fixes trac issue #8718. TODO1: calculate_operand_dims_count is almost identical code, should they be merged and its usages check for overflow? TODO2: the -1 return value seems questionable to me, but is aligned with the return value used for malloc failure. Probably both ought to be changed to AVERROR(ENOMEM). Signed-off-by: Reimar Döffinger --- libavfilter/dnn/dnn_backend_native.c | 10 +++++++++- libavfilter/dnn/dnn_backend_native.h | 2 ++ libavfilter/dnn/dnn_backend_native_layer_conv2d.c | 2 ++ libavfilter/dnn/dnn_backend_native_layer_depth2space.c | 2 ++ libavfilter/dnn/dnn_backend_native_layer_mathbinary.c | 2 ++ libavfilter/dnn/dnn_backend_native_layer_mathunary.c | 2 ++ libavfilter/dnn/dnn_backend_native_layer_maximum.c | 2 ++ libavfilter/dnn/dnn_backend_native_layer_pad.c | 2 ++ 8 files changed, 23 insertions(+), 1 deletion(-) -- 2.27.0 diff --git a/libavfilter/dnn/dnn_backend_native.c b/libavfilter/dnn/dnn_backend_native.c index 35236fc66f..a685efb092 100644 --- a/libavfilter/dnn/dnn_backend_native.c +++ b/libavfilter/dnn/dnn_backend_native.c @@ -79,6 +79,8 @@ static DNNReturnType set_input_output_native(void *model, DNNData *input, const av_freep(&oprd->data); oprd->length = calculate_operand_data_length(oprd); + if (oprd->length <= 0) + return DNN_ERROR; oprd->data = av_malloc(oprd->length); if (!oprd->data) return DNN_ERROR; @@ -295,7 +297,13 @@ int32_t calculate_operand_dims_count(const DnnOperand *oprd) int32_t calculate_operand_data_length(const DnnOperand* oprd) { // currently, we just support DNN_FLOAT - return oprd->dims[0] * oprd->dims[1] * oprd->dims[2] * oprd->dims[3] * sizeof(float); + uint64_t len = sizeof(float); + for (int i = 0; i < 4; i++) { + len *= oprd->dims[i]; + if (len > INT32_MAX) + return 0; + } + return len; } void ff_dnn_free_model_native(DNNModel **model) diff --git a/libavfilter/dnn/dnn_backend_native.h b/libavfilter/dnn/dnn_backend_native.h index bec63be450..62191ffe88 100644 --- a/libavfilter/dnn/dnn_backend_native.h +++ b/libavfilter/dnn/dnn_backend_native.h @@ -120,6 +120,8 @@ DNNReturnType ff_dnn_execute_model_native(const DNNModel *model, DNNData *output void ff_dnn_free_model_native(DNNModel **model); +// NOTE: User must check for error (return value <= 0) to handle +// case like integer overflow. int32_t calculate_operand_data_length(const DnnOperand *oprd); int32_t calculate_operand_dims_count(const DnnOperand *oprd); #endif diff --git a/libavfilter/dnn/dnn_backend_native_layer_conv2d.c b/libavfilter/dnn/dnn_backend_native_layer_conv2d.c index c05bb5eca9..a2202e4073 100644 --- a/libavfilter/dnn/dnn_backend_native_layer_conv2d.c +++ b/libavfilter/dnn/dnn_backend_native_layer_conv2d.c @@ -113,6 +113,8 @@ int dnn_execute_layer_conv2d(DnnOperand *operands, const int32_t *input_operand_ output_operand->dims[3] = conv_params->output_num; output_operand->data_type = operands[input_operand_index].data_type; output_operand->length = calculate_operand_data_length(output_operand); + if (output_operand->length <= 0) + return -1; output_operand->data = av_realloc(output_operand->data, output_operand->length); if (!output_operand->data) return -1; diff --git a/libavfilter/dnn/dnn_backend_native_layer_depth2space.c b/libavfilter/dnn/dnn_backend_native_layer_depth2space.c index 324871ceca..2c8bddf23d 100644 --- a/libavfilter/dnn/dnn_backend_native_layer_depth2space.c +++ b/libavfilter/dnn/dnn_backend_native_layer_depth2space.c @@ -75,6 +75,8 @@ int dnn_execute_layer_depth2space(DnnOperand *operands, const int32_t *input_ope output_operand->dims[3] = new_channels; output_operand->data_type = operands[input_operand_index].data_type; output_operand->length = calculate_operand_data_length(output_operand); + if (output_operand->length <= 0) + return -1; output_operand->data = av_realloc(output_operand->data, output_operand->length); if (!output_operand->data) return -1; diff --git a/libavfilter/dnn/dnn_backend_native_layer_mathbinary.c b/libavfilter/dnn/dnn_backend_native_layer_mathbinary.c index b239a20058..dd42c329a9 100644 --- a/libavfilter/dnn/dnn_backend_native_layer_mathbinary.c +++ b/libavfilter/dnn/dnn_backend_native_layer_mathbinary.c @@ -91,6 +91,8 @@ int dnn_execute_layer_math_binary(DnnOperand *operands, const int32_t *input_ope output->data_type = input->data_type; output->length = calculate_operand_data_length(output); + if (output->length <= 0) + return DNN_ERROR; output->data = av_realloc(output->data, output->length); if (!output->data) return DNN_ERROR; diff --git a/libavfilter/dnn/dnn_backend_native_layer_mathunary.c b/libavfilter/dnn/dnn_backend_native_layer_mathunary.c index 42615c43d5..8a4bd3c600 100644 --- a/libavfilter/dnn/dnn_backend_native_layer_mathunary.c +++ b/libavfilter/dnn/dnn_backend_native_layer_mathunary.c @@ -67,6 +67,8 @@ int dnn_execute_layer_math_unary(DnnOperand *operands, const int32_t *input_oper output->data_type = input->data_type; output->length = calculate_operand_data_length(output); + if (output->length <= 0) + return DNN_ERROR; output->data = av_realloc(output->data, output->length); if (!output->data) return DNN_ERROR; diff --git a/libavfilter/dnn/dnn_backend_native_layer_maximum.c b/libavfilter/dnn/dnn_backend_native_layer_maximum.c index af16e08b95..cdddfdd87b 100644 --- a/libavfilter/dnn/dnn_backend_native_layer_maximum.c +++ b/libavfilter/dnn/dnn_backend_native_layer_maximum.c @@ -64,6 +64,8 @@ int dnn_execute_layer_maximum(DnnOperand *operands, const int32_t *input_operand output->data_type = input->data_type; output->length = calculate_operand_data_length(output); + if (output->length <= 0) + return DNN_ERROR; output->data = av_realloc(output->data, output->length); if (!output->data) return DNN_ERROR; diff --git a/libavfilter/dnn/dnn_backend_native_layer_pad.c b/libavfilter/dnn/dnn_backend_native_layer_pad.c index dfbd204456..feaab001e8 100644 --- a/libavfilter/dnn/dnn_backend_native_layer_pad.c +++ b/libavfilter/dnn/dnn_backend_native_layer_pad.c @@ -111,6 +111,8 @@ int dnn_execute_layer_pad(DnnOperand *operands, const int32_t *input_operand_ind output_operand->dims[3] = new_channel; output_operand->data_type = operands[input_operand_index].data_type; output_operand->length = calculate_operand_data_length(output_operand); + if (output_operand->length <= 0) + return -1; output_operand->data = av_realloc(output_operand->data, output_operand->length); if (!output_operand->data) return -1;