[Lldb-commits] [lldb] r316740 - Fix a use-after-free in lldb-server

[Zachary Turner via lldb-commits]

It's worth mentioning again that LLVM uses StringRef pretty pervasively and
doesn't have issues.  There's nothing fundamentally different about
debuggers that makes StringRefs bad whereas they can be good in other types
of software.  If we really wanted to avoid use-after-frees we would pick a
language other than C++.

Note that StringRef is now part of the C++ standard, under the name
std::string_view.  Every part of C++ can shoot you in the foot if used
incorrectly.  This is equally true for std::string , and pointers in
general.  I don't see anything special about this particular bug.  Someone
returned a pointer to stack memory.  That's a pretty common class of bug.

Actually though, I did think of another thing we learn from this example.
We should be running LLDB under ASAN

On Tue, Oct 31, 2017 at 8:12 AM Zachary Turner <zturner at google.com> wrote:

> The takeaway from this example is nothing we don't already know.  We need
> better test coverage.
>
> On Tue, Oct 31, 2017 at 8:08 AM Greg Clayton via lldb-commits <
> lldb-commits at lists.llvm.org> wrote:
>
>> This is one example of how StringRef causes issues because it was adopted
>> everywhere. Is there any way we can change our functions so we can't run
>> into this issue? Anything we can learn from this example?
>>
>>
>>
>> > On Oct 26, 2017, at 9:53 PM, Pavel Labath via lldb-commits <
>> lldb-commits at lists.llvm.org> wrote:
>> >
>> > Author: labath
>> > Date: Thu Oct 26 21:53:24 2017
>> > New Revision: 316740
>> >
>> > URL: http://llvm.org/viewvc/llvm-project?rev=316740&view=rev
>> > Log:
>> > Fix a use-after-free in lldb-server
>> >
>> > UriParser::Parse is returning a StringRef pointing the the parsed
>> > string, but we were calling it with a temporary string. Change this to a
>> > local variable to make sure the string persists as long as we need it.
>> >
>> > Modified:
>> >
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
>> >
>> > Modified:
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
>> > URL:
>> http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp?rev=316740&r1=316739&r2=316740&view=diff
>> >
>> ==============================================================================
>> > ---
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
>> (original)
>> > +++
>> lldb/trunk/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationServerPlatform.cpp
>> Thu Oct 26 21:53:24 2017
>> > @@ -128,8 +128,9 @@ Status GDBRemoteCommunicationServerPlatf
>> >   llvm::StringRef platform_ip;
>> >   int platform_port;
>> >   llvm::StringRef platform_path;
>> > -  bool ok = UriParser::Parse(GetConnection()->GetURI(),
>> platform_scheme,
>> > -                             platform_ip, platform_port,
>> platform_path);
>> > +  std::string platform_uri = GetConnection()->GetURI();
>> > +  bool ok = UriParser::Parse(platform_uri, platform_scheme,
>> platform_ip,
>> > +                             platform_port, platform_path);
>> >   UNUSED_IF_ASSERT_DISABLED(ok);
>> >   assert(ok);
>> >
>> >
>> >
>> > _______________________________________________
>> > lldb-commits mailing list
>> > lldb-commits at lists.llvm.org
>> > http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>> _______________________________________________
>> lldb-commits mailing list
>> lldb-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20171031/54610d14/attachment-0001.html>