From patchwork Mon Jul 6 06:59:25 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: 20838 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 84E11449BE1 for ; Mon, 6 Jul 2020 10:19:41 +0300 (EEST) Received: from [127.0.1.1] (localhost [127.0.0.1]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTP id 5876A68B0A5; Mon, 6 Jul 2020 10:19:41 +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.13]) by ffbox0-bg.mplayerhq.hu (Postfix) with ESMTPS id 84C91689C7E for ; Mon, 6 Jul 2020 10:19:35 +0300 (EEST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=gmx.net; s=badeba3b8450; t=1594019975; bh=ayH9x9yEbF1bDni1/7zwRK0HPM+SJwa5W+GgNvQt6+o=; h=X-UI-Sender-Class:From:To:Subject:Date; b=Gqp3SsoEn6YEfsPNN2E2d7mWw5ZEm17Ep8iKpRU7Hppom1RWNj2I1ndNTMPLkkef7 kgYhWoLSlL+3+0aSM8jMhkO+PX40AJ2cuRXJ8aVgr68i29kxe3d/Sb5caFFpKYHtdj OeUiEvdM8fdzLRh9sAsfIZCIEebE/Y82tbFsMrNQ= X-UI-Sender-Class: 01bb95c1-4bf8-414a-932a-4f6e2808ef9c Received: from localhost ([81.170.149.220]) by mail.gmx.com (mrgmx005 [212.227.17.190]) with ESMTPSA (Nemesis) id 1MZCb5-1kNOUJ14Ub-00V5Oi for ; Mon, 06 Jul 2020 08:59:26 +0200 From: =?utf-8?q?Reimar_D=C3=B6ffinger?= To: ffmpeg-devel@ffmpeg.org Date: Mon, 6 Jul 2020 08:59:25 +0200 Message-Id: <20200706065925.4814-1-Reimar.Doeffinger@gmx.de> X-Mailer: git-send-email 2.27.0 MIME-Version: 1.0 X-Provags-ID: V03:K1:bsdAbG4NrLen2Ccy6XHTZTo0aNbSLJfQVjxFCYODJKX2rs0EC1Z +2Gh+QEgw/A8ZlrnaFp+Ex6urBFXVvhR7qsqtQhboqniu6sFebZzY7m30t+ORcG0WDperzK 2ZADRuQtt7YQEhgoHh7P6MT0z7IEaT0Z6BszTTswqdcSD+ia1T7iXeZ3VdhDYYjOs+Eewsm 6e/5E3raQDXbDsyOFyfbw== X-Spam-Flag: YES X-UI-Out-Filterresults: junk:10;V03:K0:ELsOzAdi7nk=:uL2FU8UBg76rS2VxIiTPgsuj k2t7qgdb6iNaDKz0TgMdkF0VIva2WahFyoaRD25oM3tNQ0fXRpAwdaOQ2MHX3ovn8X1XaCbAk 2Lv/zNiyTQRxWE96WGQz5IkTokVP28W+dGhRx9G1Xzrf//3w4THwT2l7DkmZnrNhkIHq7b6L1 4RTvRS4TMY8izDx1fH/L8AGaRNUI7AXQ/IalvN/3sFM1ysAf77dgsevP8UrnwUGt7uRZZRyA1 NdRcl1eyTMF5SkdS8O4lrvJqv1/NX4ZWxzrxRJkQun6NihfayJIUZh6c7KrrldNhhHTstzKEQ fzTLB89e1O/XEDxV8+2cVi/cVdhm8gYU9HpINbJdt2XrrAL7zmkXs1E8fqElAqXxn8FHzK+md 0795kx1nzh3Bcj3yZ9J0IrGNDeyX0M4Rydm0FyKYSX2mpnlufFkNPpbkbvLYcY0kvfUtuf+us 8wkzTkjbm3YZVKKShA3qkC2fmR9iL39HmsD7Wb0HWbTpVRcqCWfkPr15QsjWnZj/G3pitZgH/ +daq7C0ueka8QSgIenNPJ2LC0hsfHZQ6DpCjjD8WSXaZTNhUb9y1Ns67aShto5/Dsp1gbRxc3 wl8mFupu9KyeoDeoDIP+9yNvL3Ta7F/JlPM96QvPcJJz7099Xgjv1i6dHslPVJxm4OYQYuYjz FD2YGWUUMAOiOxS5fmvrTunII5iaPM6qO1pcw5gTFvpfJsSEllsmrImtGZb9xPdM2npddUYFA QnbB2qcEvUoAWlWK06x8LdkOSDbfx3GPq6l6Je/bU/ytWK3ZOGrS/fFe6Je2wJV/g9mwY6Foo 5I4ni27SCIYoJmiYxaUJJBl2oUONHQXhdRv0J9+1NpiRgBSrlCcNYtAMjYy0HTyUKFK7S6N3h taHvks7HkzemqmNN3mCV9myiBAjD4ox6GMxTREoljdr9N4qcL/5PwVtS2+BlnKO5T159edVPQ BQtlv4NWinyPsNd/euYGOeeZWhbyeO5BRecIej/etsa6xJxUUyNgjDIpoa2aXpISQDmnR39bV PDjxbcLb0F9OAm71MhtzgRt7pOPWFWvq30O6ZceSLP6Jpy7HAkfx6saP5/QVg2gHLCafZCk4x ChqxCgIpOe10ogm/l7WA+4Mapbf2VoZkBXKMFKLf5c0XsGgyHpQqjOD8ZSOFKXYFb+p3+57VQ WNm07hJg55m/zIlOXSmCsccXS0PRXp3S839MY4l4vSCNROFq2FdmdaUnDSOPtOEXvhoWZraNn kzoJPI29un3r36gybpAsm4gJ7TXSaO98AJZXg== 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..bb30ccf154 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) + 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..d82ef9486e 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) + 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;