bug-commoncpp
[Top][All Lists]
Advanced
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Segmentation fault//memcpy misuse with command options (partial fix

From: David Sugar
Subject: Re: Segmentation fault//memcpy misuse with command options (partial fix patch included)
Date: Tue, 13 Apr 2004 14:55:15 -0400
User-agent: KMail/1.6.2 
I have been able to duplicate this problem here, using a different example, 
and I know what is happening.  When the slot list of linked deleted 
(reusable) buffers is formed, each buffer has to be unique.  I believe 
somewhere we are permitting other strings to hold a duplicate copy of the 
String internals and buffer pointers, and hence the string buffer in the 
linked list is being overwritten by another string object that also has it's 
buffer.  Since the linked list is formed by making the buffer content into 
link pointers, these links are getting overriden by the other string object 
that still has a pointer to the same piece of memory that is thought to be 
free, and hence starts containing pointers that are not links but rather 
composed of character data.

I believe the likely culpret is ::copy(), which allows "content" structure to 
= the content of the original string unmodified when !isBig(), and here is 
where we may copy the alias to the same memory pointer, but I need to do some 
further testing to see if it is indeed there or in another one of the 
operations...

On Saturday 10 April 2004 05:58 am, David Sugar wrote:
> My first thought is that most of these operations depend on getLength
> returning the correct string length (in effect strlen() + 1 null byte).
> However, the String class actually stores a length field, which is what
> getLength() actually returns, and maybe that length field is not being kept
> in sync with the actual string length in some circumstances!  It would be
> interesting to compare the length returned by getLength with the length
> returned by strlen() in front of each of these memmove's...
>
> On Friday 09 April 2004 05:00 pm, you wrote:
> > In addition, none of these problems occur with commoncpp2 1.0.13 only in
> > 1.1.2.
> >
> > Joshua Moore-Oliva.
> >
> > On April 8, 2004 9:19 pm, David Sugar wrote:
> > > So what you did is basically replace every memcpy with a memmove?
> > > Hmm...in at least some cases, memcpy may be inline'd with assembler
> > > code. Perhaps there is something odd in the inline code substitution? 
> > > Also, your patch got a little chopped up in the debugging text...
> > >
> > > David
> > >
> > > On Friday 09 April 2004 04:02 am, Joshua Moore-Oliva wrote:
> > > > I created a program with quite a few options, and I got a
> > > > segmentation fault that appears to be a mis-use of memcpy.
> > > >
> > > > Included is a test program to reproduce the problem and some
> > > > debugging output.
> > > >
> > > > I was able to remove the problems with memcpy with the changes in the
> > > > included patch, however I am not familiar enough with the code to
> > > > determine why the getSpace function in the String library is going
> > > > out of bounds...
> > > >
> > > > debugging output before patch
> > > >
> > > > ==17692== Source and destination overlap in memcpy(0x41E1A83C,
> > > > 0x41E1A87C, 86) ==17692==    at 0x40022224: memcpy (in
> > > > /usr/lib/valgrind/vgskin_memcheck.so) ==17692==    by 0x40277A94:
> > > > ost::String::set(char const*, unsigned) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==17692==    by 0x40277B07:
> > > > ost::String::set(ost::String const&) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==17692==    by 0x402B16C1:
> > > > ost::String::operator=(ost::String const&) (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x402B7B10:
> > > > ost::CommandOptionParse_impl::makePrintErrors() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x402B6CAC:
> > > > ost::CommandOptionParse_impl::printErrors() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x8048BD8: main (in
> > > > /home/chatgris/a.out)
> > > > ==17692==
> > > > ==17692== Source and destination overlap in memcpy(0x41E1A87C,
> > > > 0x41E1A83C, 87) ==17692==    at 0x40022224: memcpy (in
> > > > /usr/lib/valgrind/vgskin_memcheck.so) ==17692==    by 0x40277C75:
> > > > ost::String::copy(ost::String const&) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==17692==    by 0x4027647B:
> > > > ost::String::String(ost::String const&) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==17692==    by 0x4027887C:
> > > > ost::operator+(ost::String const&, char const*) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==17692==    by 0x402B7B83:
> > > > ost::CommandOptionParse_impl::makePrintErrors() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x402B6CAC:
> > > > ost::CommandOptionParse_impl::printErrors() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x8048BD8: main (in
> > > > /home/chatgris/a.out)
> > > > ==17692==
> > > > ==17692== Source and destination overlap in memcpy(0x41E1AC68,
> > > > 0x41E1ABA8, 215) ==17692==    at 0x40022224: memcpy (in
> > > > /usr/lib/valgrind/vgskin_memcheck.so) ==17692==    by 0x40278310:
> > > > ost::String::resize(unsigned) (in /usr/lib/libccgnu2-1.1.so.0.0.2)
> > > > ==17692==    by 0x40277A71: ost::String::set(char const*, unsigned)
> > > > (in /usr/lib/libccgnu2-1.1.so.0.0.2) ==17692==    by 0x40277B07:
> > > > ost::String::set(ost::String const&) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==17692==    by 0x402B16C1:
> > > > ost::String::operator=(ost::String const&) (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x402B7DD5:
> > > > ost::CommandOptionParse_impl::makePrintErrors() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x402B6CAC:
> > > > ost::CommandOptionParse_impl::printErrors() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x8048BD8: main (in
> > > > /home/chatgris/a.out)
> > > > Value required for option '--connection_string' is missing
> > > > Value==17692==
> > > > ==17692== Source and destination overlap in memcpy(0x41E1A7BC,
> > > > 0x41E1A7DC, 41) ==17692==    at 0x40022224: memcpy (in
> > > > /usr/lib/valgrind/vgskin_memcheck.so) ==17692==    by 0x40277C75:
> > > > ost::String::copy(ost::String const&) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==17692==    by 0x4027647B:
> > > > ost::String::String(ost::String const&) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==17692==    by 0x402787B2:
> > > > ost::operator+(ost::String const&, char) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==17692==    by 0x402B8015:
> > > > ost::CommandOptionParse_impl::makePrintUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x402B6C7A:
> > > > ost::CommandOptionParse_impl::printUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x8048BFA: main (in
> > > > /home/chatgris/a.out)
> > > > ==17692==
> > > > ==17692== Invalid read of size 4
> > > > ==17692==    at 0x40278576: ost::String::getSpace(unsigned) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==17692==    by 0x402781F6:
> > > > ost::String::resize(unsigned) (in /usr/lib/libccgnu2-1.1.so.0.0.2)
> > > > ==17692==    by 0x40277A71: ost::String::set(char const*, unsigned)
> > > > (in /usr/lib/libccgnu2-1.1.so.0.0.2) ==17692==    by 0x40277B07:
> > > > ost::String::set(ost::String const&) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==17692==    by 0x402B16C1:
> > > > oValue==26722== Invalid read of size 4 ==26722==    at 0x40278576:
> > > > ost::String::getSpace(unsigned) (in /usr/lib/libccgnu2-1.1.so.0.0.2)
> > > > ==26722==    by 0x402781F6:
> > > > ost::String::resize(unsigned) (in /usr/lib/libccgnu2-1.1.so.0.0.2)
> > > > ==26722==    by 0x40277A71: ost::String::set(char const*, unsigned)
> > > > (in /usr/lib/libccgnu2-1.1.so.0.0.2) ==26722==    by 0x40277B07:
> > > > ost::String::set(ost::String const&) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==26722==    by 0x402B16C1:
> > > > ost::String::operator=(ost::String const&) (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x402B802A:
> > > > ost::CommandOptionParse_impl::makePrintUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x402B6C7A:
> > > > ost::CommandOptionParse_impl::printUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x8048BFA: main
> > > > (main.cpp:69)
> > > > ==26722==  Address 0x20202020 is not stack'd, malloc'd or free'd
> > > > ==26722==
> > > > ==26722== Process terminating with default action of signal 11
> > > > (SIGSEGV): dumping core ==26722==  Address not mapped to object at
> > > > address 0x20202020 ==26722==    at 0x40278576:
> > > > ost::String::getSpace(unsigned) (in /usr/lib/libccgnu2-1.1.so.0.0.2)
> > > > ==26722==    by 0x402781F6:
> > > > ost::String::resize(unsigned) (in /usr/lib/libccgnu2-1.1.so.0.0.2)
> > > > ==26722==    by 0x40277A71: ost::String::set(char const*, unsigned)
> > > > (in /usr/lib/libccgnu2-1.1.so.0.0.2) ==26722==    by 0x40277B07:
> > > > ost::String::set(ost::String const&) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==26722==    by 0x402B16C1:
> > > > ost::String::operator=(ost::String const&) (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x402B802A:
> > > > ost::CommandOptionParse_impl::makePrintUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x402B6C7A:
> > > > ost::CommandOptionParse_impl::printUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x8048BFA: main
> > > > (main.cpp:69)st::String::operator=(ost::String const&) (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x402B802A:
> > > > ost::CommandOptionParse_impl::makePrintUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x402B6C7A:
> > > > ost::CommandOptionParse_impl::printUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x8048BFA: main (in
> > > > /home/chatgris/a.out)
> > > > ==17692==  Address 0x20202020 is not stack'd, malloc'd or free'd
> > > > ==17692==
> > > > ==17692== Process terminating with default action of signal 11
> > > > (SIGSEGV): dumping core ==17692==  Address not mapped to object at
> > > > address 0x20202020 ==17692==    at 0x40278576:
> > > > ost::String::getSpace(unsigned) (in /usr/lib/libccgnu2-1.1.so.0.0.2)
> > > > ==17692==    by 0x402781F6:
> > > > ost::String::resize(unsigned) (in /usr/lib/libccgnu2-1.1.so.0.0.2)
> > > > ==17692==    by 0x40277A71: ost::String::set(char const*, unsigned)
> > > > (in /usr/lib/libccgnu2-1.1.so.0.0.2) ==17692==    by 0x40277B07:
> > > > ost::String::set(ost::String const&) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==17692==    by 0x402B16C1:
> > > > ost::String::operator=(ost::String const&) (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x402B802A:
> > > > ost::CommandOptionParse_impl::makePrintUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x402B6C7A:
> > > > ost::CommandOptionParse_impl::printUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==17692==    by 0x8048BFA: main (in
> > > > /home/chatgris/a.out)
> > > >
> > > >
> > > > test case
> > > >
> > > > #include <iostream>
> > > > #include <cc++/common.h>
> > > >
> > > > ost::CommandOptionArg optConnInfo (
> > > >   "connection_string", "c", "Connection String.", true
> > > > );
> > > >
> > > > ost::CommandOptionArg optMergeDelimiterField (
> > > >   "field_delimiter", "f", "Field delimiter for merge information",
> > > > true );
> > > >
> > > > ost::CommandOptionArg optMergeDelimiterRecord (
> > > >   "record_delimiter", "r", "Record delimiter for merge information",
> > > > true );
> > > >
> > > > ost::CommandOptionArg optMergeNames (
> > > >   "merge_names", "n", "Merge Field Names (tab delimited)", true
> > > > );
> > > >
> > > > ost::CommandOptionArg optMessageSource (
> > > >   "message_source", "s", "Message Source", true
> > > > );
> > > >
> > > > ost::CommandOptionArg optQuery (
> > > >   "query", "q", "Query to retrieve send data", true
> > > > );
> > > >
> > > > ost::CommandOptionArg optSortDir (
> > > >   "sort_dir", "sd", "Directory where the sort binary resides", true
> > > > );
> > > >
> > > > ost::CommandOptionArg optSortName (
> > > >   "sort_name", "sn", "Name of the sort binary", true
> > > > );
> > > >
> > > > ost::CommandOptionArg optSortTmpDir (
> > > >   "sort_tmp_dir", "st", "Temp dir for sort to use", true
> > > > );
> > > >
> > > > ost::CommandOptionArg optMaxBatchSize (
> > > >   "max_batch_size", "b", "Maximum Batch Size", true
> > > > );
> > > >
> > > > ost::CommandOptionArg optPriority (
> > > >   "priority", "p", "Priority for mailing", true
> > > > );
> > > >
> > > > ost::CommandOptionNoArg optHelpArg (
> > > >   "help", "?", "Print help usage"
> > > > );
> > > >
> > > > int main ( int argc, char ** argv ) {
> > > >   ost::CommandOptionParse * optArgs =
> > > >     ost::makeCommandOptionParse ( argc
> > > >                                 , argv
> > > >                                 , "Configurable Procmail receiver.\n"
> > > >   );
> > > >
> > > >   // If the user requested help then suppress all the usage error
> > > >   // messages.
> > > >   if ( optHelpArg.numSet ) {
> > > >     std::cerr << optArgs->printUsage();
> > > >     return 0;
> > > >   }
> > > >
> > > >   // Print usage your way.
> > > >   if ( optArgs->argsHaveError() ) {
> > > >     std::cerr << optArgs->printErrors();
> > > >     std::cerr << optArgs->printUsage();
> > > >     return 1;
> > > >   }
> > > >
> > > >   std::cout << "Success" << std::endl;
> > > >
> > > >   delete optArgs;
> > > >
> > > >   return 0;
> > > > }
> > > >
> > > > Patch to get rid of memcpy
> > > >
> > > > 170c170
> > > > <               memmove(ptr + start, str, count);
> > > > ---
> > > >
> > > > >               memcpy(ptr + start, str, count);
> > > >
> > > > 218c218
> > > > <       memmove(ptr + start, ptr + start + count, len - start -
> > > > count); ---
> > > >
> > > > >       memcpy(ptr + start, ptr + start + count, len - start -
> > > > > count);
> > > >
> > > > 253c253
> > > > <       memmove(getText() + getLength(), str, len);
> > > > ---
> > > >
> > > > >       memcpy(getText() + getLength(), str, len);
> > > >
> > > > 391c391
> > > > <       memmove(getText(), str, Value==26722== Invalid read of size 4
> > > > ==26722==    at 0x40278576: ost::String::getSpace(unsigned) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==26722==    by 0x402781F6:
> > > > ost::String::resize(unsigned) (in /usr/lib/libccgnu2-1.1.so.0.0.2)
> > > > ==26722==    by 0x40277A71: ost::String::set(char const*, unsigned)
> > > > (in /usr/lib/libccgnu2-1.1.so.0.0.2) ==26722==    by 0x40277B07:
> > > > ost::String::set(ost::String const&) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==26722==    by 0x402B16C1:
> > > > ost::String::operator=(ost::String const&) (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x402B802A:
> > > > ost::CommandOptionParse_impl::makePrintUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x402B6C7A:
> > > > ost::CommandOptionParse_impl::printUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x8048BFA: main
> > > > (main.cpp:69)
> > > > ==26722==  Address 0x20202020 is not stack'd, malloc'd or free'd
> > > > ==26722==
> > > > ==26722== Process terminating with default action of signal 11
> > > > (SIGSEGV): dumping core ==26722==  Address not mapped to object at
> > > > address 0x20202020 ==26722==    at 0x40278576:
> > > > ost::String::getSpace(unsigned) (in /usr/lib/libccgnu2-1.1.so.0.0.2)
> > > > ==26722==    by 0x402781F6:
> > > > ost::String::resize(unsigned) (in /usr/lib/libccgnu2-1.1.so.0.0.2)
> > > > ==26722==    by 0x40277A71: ost::String::set(char const*, unsigned)
> > > > (in /usr/lib/libccgnu2-1.1.so.0.0.2) ==26722==    by 0x40277B07:
> > > > ost::String::set(ost::String const&) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==26722==    by 0x402B16C1:
> > > > ost::String::operator=(ost::String const&) (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x402B802A:
> > > > ost::CommandOptionParse_impl::makePrintUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x402B6C7A:
> > > > ost::CommandOptionParse_impl::printUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x8048BFA: main
> > > > (main.cpp:69)len);
> > > > ---
> > > >
> > > > >       memcpy(getText(), str, len);
> > > >
> > > > 418c418
> > > > <               memmove(content.ministring.text, getText(),
> > > > getLength() + 1); ---
> > > >
> > > > >               memcpy(content.ministring.text, getText(),
> > > > > getLength() + 1);
> > > >
> > > > 426c426
> > > > <       memmove(content.bigstring.text, ptr, getLength() + 1);
> > > > ---
> > > >
> > > > >       memcpy(content.bigstring.text, ptr, getLength() + 1);
> > > >
> > > > 576c576
> > > > <               memmove(ptr, content.ministring.text, length);
> > > > ---
> > > >
> > > > >               memcpy(ptr, content.ministring.text, length);
> > > >
> > > > 588c588
> > > > <               memmove(content.ministring.text, ptr, length);
> > > > ---
> > > >
> > > > >               memcpy(content.ministring.text, ptr, length);
> > > >
> > > > 597c597
> > > > <       memmove(ptr, getText(), length);
> > > > ---
> > > >
> > > > >       memcpy(ptr, getText(), length);
> > > >
> > > > Debugging results of above program after patch applied
> > > >
> > > > Value==26722== Invalid read of size 4
> > > > ==26722==    at 0x40278576: ost::String::getSpace(unsigned) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==26722==    by 0x402781F6:
> > > > ost::String::resize(unsigned) (in /usr/lib/libccgnu2-1.1.so.0.0.2)
> > > > ==26722==    by 0x40277A71: ost::String::set(char const*, unsigned)
> > > > (in /usr/lib/libccgnu2-1.1.so.0.0.2) ==26722==    by 0x40277B07:
> > > > ost::String::set(ost::String const&) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==26722==    by 0x402B16C1:
> > > > ost::String::operator=(ost::String const&) (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x402B802A:
> > > > ost::CommandOptionParse_impl::makePrintUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x402B6C7A:
> > > > ost::CommandOptionParse_impl::printUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x8048BFA: main
> > > > (main.cpp:69)
> > > > ==26722==  Address 0x20202020 is not stack'd, malloc'd or free'd
> > > > ==26722==
> > > > ==26722== Process terminating with default action of signal 11
> > > > (SIGSEGV): dumping core ==26722==  Address not mapped to object at
> > > > address 0x20202020 ==26722==    at 0x40278576:
> > > > ost::String::getSpace(unsigned) (in /usr/lib/libccgnu2-1.1.so.0.0.2)
> > > > ==26722==    by 0x402781F6:
> > > > ost::String::resize(unsigned) (in /usr/lib/libccgnu2-1.1.so.0.0.2)
> > > > ==26722==    by 0x40277A71: ost::String::set(char const*, unsigned)
> > > > (in /usr/lib/libccgnu2-1.1.so.0.0.2) ==26722==    by 0x40277B07:
> > > > ost::String::set(ost::String const&) (in
> > > > /usr/lib/libccgnu2-1.1.so.0.0.2) ==26722==    by 0x402B16C1:
> > > > ost::String::operator=(ost::String const&) (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x402B802A:
> > > > ost::CommandOptionParse_impl::makePrintUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x402B6C7A:
> > > > ost::CommandOptionParse_impl::printUsage() (in
> > > > /usr/lib/libccext2-1.1.so.0.0.2) ==26722==    by 0x8048BFA: main
> > > > (main.cpp:69)
> > > >
> > > >
> > > > _______________________________________________
> > > > Bug-commoncpp mailing list
> > > > address@hidden
> > > > http://mail.gnu.org/mailman/listinfo/bug-commoncpp
>
> _______________________________________________
> Bug-commoncpp mailing list
> address@hidden
> http://mail.gnu.org/mailman/listinfo/bug-commoncpp
reply via email to
[Prev in Thread] Current Thread [Next in Thread]