From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp3.mail.ru (smtp3.mail.ru [94.100.179.58]) (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 0312546970F for ; Tue, 26 Nov 2019 02:02:40 +0300 (MSK) References: <20191125162701.82576-1-sergepetrenko@tarantool.org> From: Vladislav Shpilevoy Message-ID: Date: Tue, 26 Nov 2019 00:02:39 +0100 MIME-Version: 1.0 In-Reply-To: <20191125162701.82576-1-sergepetrenko@tarantool.org> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Subject: Re: [Tarantool-patches] [PATCH] backtrace: fix out of bounds access on backtrace printing List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Serge Petrenko Cc: tarantool-patches@dev.tarantool.org Hi! Thanks for the patch! Perhaps the zero termination was done for a case, when the cycle in backtrace() does not run even one iteration. For example, if unw_step() returns an error. Then the buffer is not terminated. So I think it is better to keep *p = 0, but do it before the cycle. On 25/11/2019 17:27, Serge Petrenko wrote: > snrpintf always null-terminates the passed string, and it also returns > the number of bytes that "would have been written if there was enough > space", so not only we don't have to null-terminate the string, but even > more so we shouldn't do it erroneously. > > Closes #4636 > --- > https://github.com/tarantool/tarantool/issues/4636 > https://github.com/tarantool/tarantool/tree/sp/gh-4636-bt-print-fix > > src/lib/core/backtrace.cc | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/src/lib/core/backtrace.cc b/src/lib/core/backtrace.cc > index 57e541c25..903ffb79c 100644 > --- a/src/lib/core/backtrace.cc > +++ b/src/lib/core/backtrace.cc > @@ -173,7 +173,6 @@ backtrace() > say_debug("unwinding error: %i", unw_status); > #endif > out: > - *p = '\0'; > return backtrace_buf; > } > >