Fix test (dentry->d_name is invalidated by closedir...) (!41)

We have to copy the dentry->d_name before calling closedir().  If not,
the string points to nothing and the test fails (It does not fail all
the time, but only by chance).

Confirmed using valgrind.  Selected output from running the unit test
under valgrind:

  $ valgrind --track-origins=yes ./test_blockSpecial
  ==25110== Memcheck, a memory error detector
  ...
  ==25110== Command: ./test_BlockSpecial
  ==25110==
  Running main() from src/gtest_main.cc
  [==========] Running 26 tests from 1 test case.
  [----------] Global test environment set-up.
  [----------] 26 tests from BlockSpecialTest
  ...
  [ RUN      ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches
  ==25110== Invalid read of size 1
  ==25110==    at 0x4C2C9B2: strlen (vg_replace_strmem.c:458)
  ==25110==    by 0x40E7C4: length (char_traits.h:259)
  ==25110==    by 0x40E7C4: append (basic_string.h:1009)
  ==25110==    by 0x40E7C4: operator+<char, std::char_traits<char>, std::allocator<char> > (basic_string.h:2468)
  ==25110==    by 0x40E7C4: get_link_name (test_BlockSpecial.cc:156)
  ==25110==    by 0x40E7C4: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
  ...
  =25110==  Address 0x1231ea93 is 115 bytes inside a block of size 32,816 free'd
  ==25110==    at 0x4C2ACBD: free (vg_replace_malloc.c:530)
  ==25110==    by 0x9F773AC: closedir (in /usr/lib64/libc-2.17.so)
  ==25110==    by 0x40E7AC: get_link_name (test_BlockSpecial.cc:153)
  ==25110==    by 0x40E7AC: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
  ...
  ==25110==  Block was alloc'd at
  ==25110==    at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
  ==25110==    by 0x9F77280: __alloc_dir (in /usr/lib64/libc-2.17.so)
  ==25110==    by 0x40E746: get_link_name (test_BlockSpecial.cc:134)
  ==25110==    by 0x40E746: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
  ...
  ==25110== Invalid read of size 1
  ==25110==    at 0x4C2E220: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:1022)
  ==25110==    by 0x953A997: std::string::append(char const*, unsigned long) (in /usr/lib64/libstdc++.so.6.0.19)
  ==25110==    by 0x40E7D2: append (basic_string.h:1009)
  ==25110==    by 0x40E7D2: operator+<char, std::char_traits<char>, std::allocator<char> > (basic_string.h:2468)
  ==25110==    by 0x40E7D2: get_link_name (test_BlockSpecial.cc:156)
  ==25110==    by 0x40E7D2: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
  ...
  ==25110==  Address 0x1231ea93 is 115 bytes inside a block of size 32,816 free'd
  ==25110==    at 0x4C2ACBD: free (vg_replace_malloc.c:530)
  ==25110==    by 0x9F773AC: closedir (in /usr/lib64/libc-2.17.so)
  ==25110==    by 0x40E7AC: get_link_name (test_BlockSpecial.cc:153)
  ==25110==    by 0x40E7AC: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
  ...
  ==25110==  Block was alloc'd at
  ==25110==    at 0x4C29BC3: malloc (vg_replace_malloc.c:299)
  ==25110==    by 0x9F77280: __alloc_dir (in /usr/lib64/libc-2.17.so)
  ==25110==    by 0x40E746: get_link_name (test_BlockSpecial.cc:134)
  ==25110==    by 0x40E746: GParted::BlockSpecialTest_NamedBlockSpecialObjectBySymlinkMatches_Test::TestBody() (test_BlockSpecial.cc:247)
  ...
  [       OK ] BlockSpecialTest.NamedBlockSpecialObjectBySymlinkMatches (50 ms)

Selected lines from test_BlockSpecial.cc:

  132  static std::string get_link_name()
  133  {
  134          DIR * dir = opendir( "/dev/disk/by-id" );
  ...
  141          bool found = false;
  142          struct dirent * dentry;
  143          // Silence GCC [-Wparentheses] warning with double parentheses
  144          while ( ( dentry = readdir( dir ) ) )
  145          {
  146                  if ( strcmp( dentry->d_name, "." )  != 0 &&
  147                       strcmp( dentry->d_name, ". " ) != 0    )
  148                  {
  149                          found = true;
  150                          break;
  151                  }
  152          }
  153          closedir( dir );
  154
  155          if ( found )
  156                  return std::string( "/dev/disk/by-id/" ) + dentry->d_name;

So the memory referred to by dentry was allocated on line 134, freed on
153 and accessed after freed on 156.

Closes !41 - Fix test (dentry->d_name is invalidated by closedir...)
This commit is contained in:
Félix Piédallu 2019-05-22 12:00:53 +02:00 committed by Mike Fleetwood
parent 0dedf8defe
commit cdba5cee35
1 changed files with 3 additions and 1 deletions

View File

@ -138,6 +138,7 @@ static std::string get_link_name()
return ""; return "";
} }
std::string name;
bool found = false; bool found = false;
struct dirent * dentry; struct dirent * dentry;
// Silence GCC [-Wparentheses] warning with double parentheses // Silence GCC [-Wparentheses] warning with double parentheses
@ -146,6 +147,7 @@ static std::string get_link_name()
if ( strcmp( dentry->d_name, "." ) != 0 && if ( strcmp( dentry->d_name, "." ) != 0 &&
strcmp( dentry->d_name, ".." ) != 0 ) strcmp( dentry->d_name, ".." ) != 0 )
{ {
name = dentry->d_name;
found = true; found = true;
break; break;
} }
@ -153,7 +155,7 @@ static std::string get_link_name()
closedir( dir ); closedir( dir );
if ( found ) if ( found )
return std::string( "/dev/disk/by-id/" ) + dentry->d_name; return std::string("/dev/disk/by-id/") + name;
ADD_FAILURE() << __func__ << "(): No entries found in directory '/dev/disk/by-id'"; ADD_FAILURE() << __func__ << "(): No entries found in directory '/dev/disk/by-id'";
return ""; return "";