From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 312596EC55; Sat, 28 Aug 2021 00:44:26 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 312596EC55 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1630100666; bh=x16oTsA5Y/CZ2SD/LqbDDf3WC6m8BFhC/00JjJRPO/Y=; h=To:Cc:References:Date:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=puB0P0ecUgXIoT1lMQBvm5nzVQEOPNnhYvtPzj5fdiA3cClBF1E3k6MF6BXvs9Taa ahQfXqml4AgGUFqC2HKM4cP81UNwI/kzpITaXYvKpEPY1cqmCfGZkXQXBsCQ/HbmIc 1h1UGapcHWhIe4hadLEDQDTQW4AVpVAX18ac60JI= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 16F0F6EC55 for ; Sat, 28 Aug 2021 00:44:25 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 16F0F6EC55 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1mJjeC-000736-6m; Sat, 28 Aug 2021 00:44:24 +0300 To: Mergen Imeev Cc: tarantool-patches@dev.tarantool.org References: <7c4620245d61624883115541490cd94d10626c00.1629976113.git.imeevma@gmail.com> <40a89d61-e252-731c-d419-6c162eecefb4@tarantool.org> <20210827152223.GA435770@tarantool.org> Message-ID: <491db4a6-2fa4-8346-c22f-0924630f7e55@tarantool.org> Date: Fri, 27 Aug 2021 23:44:23 +0200 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.13.0 MIME-Version: 1.0 In-Reply-To: <20210827152223.GA435770@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD92087353F0EC44DD9F9A2272A1D086A28553D1D5C4B4124EF182A05F538085040F76CFD4D63DC56BC7151CD0F8D5CD246708AB29E77CC71650DB0185E7C1EE49D X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7DB84ED444C624799EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637968EC5F77C2942FE8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D84B6D0B5AF2EE6743B57DAF29CA2A74DD117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC2EE5AD8F952D28FBA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18CB629EEF1311BF91D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B6A45692FFBBD75A6A089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A5032AF3FFF06980583F54E53BFAEE4E2E8DA3033812251AAFD59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75E3127721F5A72C97410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D3463DBE2ADA183F62FF6AA1C0CFA3167577B74C1FA660B1F4D713549D939F2E86EB5527936CB041BA61D7E09C32AA3244CC00425C6D804C633EF3A54A9772F65BC3E8609A02908F271729B2BEF169E0186 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojZ50fH/f1OoC7SnMqwcA1pg== X-Mailru-Sender: 689FA8AB762F7393C37E3C1AEC41BA5D246FC21614182BA112D0C28FF5E6BC5D3841015FED1DE5223CC9A89AB576DD93FB559BB5D741EB963CF37A108A312F5C27E8A8C3839CE0E267EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v1 1/1] sql: fix error on copy empty string in mem_copy() X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Thanks for the fixes! See 3 comments below. > sql: fix error on copy empty string in mem_copy() > > This patch fixes the problem with copying an empty string in mem_copy(). > Previously, because the string length was 0, an error was thrown, but > the diag was not set, which could lead to an error due to an empty diag > or to a double free. > > Closes #6157 1. You also need to add closes 6399, don't you? > diff --git a/src/box/sql/vdbeaux.c b/src/box/sql/vdbeaux.c > index 2d7800b17..beb8cee04 100644 > --- a/src/box/sql/vdbeaux.c > +++ b/src/box/sql/vdbeaux.c > @@ -2318,8 +2318,12 @@ sqlVdbeGetBoundValue(struct Vdbe *v, int iVar) > Mem *pMem = &v->aVar[iVar - 1]; > if (!mem_is_null(pMem)) { > sql_value *pRet = sqlValueNew(v->db); > - if (pRet != NULL) > - mem_copy(pRet, pMem); > + if (pRet == NULL) > + return NULL; > + if (mem_copy(pRet, pMem) != 0) { > + sqlDbFree(sql_get(), pRet); 2. Shouldn't you use sqlValueFree()? > + return NULL; > + } > return pRet; > } > } > diff --git a/test/sql-tap/gh-6157-unnecessary-free-on-string.test.lua b/test/sql-tap/gh-6157-unnecessary-free-on-string.test.lua > new file mode 100755 > index 000000000..e0c09a325 > --- /dev/null > +++ b/test/sql-tap/gh-6157-unnecessary-free-on-string.test.lua > @@ -0,0 +1,18 @@ > +#!/usr/bin/env tarantool > +local tap = require('tap') > +local test = tap.test('test wrong error in mem_copy()') > + > +-- > +-- Make sure there is no assert due to an incorrectly set error in mem_copy(). > +-- How this test works: We have 128 mempool cells in SQL ("lookaside"), and > +-- until those 128 cells are filled in, the error cannot be reproduced. Also, we > +-- have to get '' from somewhere because if we just enter it, it will be of type > +-- STATIC and no memory will be allocated. 3. You mention 128 cells, but I don't see how 128 or something close is used in this test. > +-- > +local s = "NULLIF(SUBSTR('123', 1, 0), NULL)" > +for i = 1, 5 do s = s..', '..s end > +local res = box.execute("SELECT "..s).rows[1] > + > +test:plan(1) > +test:is(#res, 32, 'wrong error in mem_copy() was set') > +os.exit(test:check() and 0 or 1)