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



commit cdba5cee35f4fa7fd515687b2d9ef8bd40d8cdd0
Author: Félix Piédallu <felix piedallu me>
Date:   Wed May 22 12:00:53 2019 +0200

    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...)

 tests/test_BlockSpecial.cc | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)
---
diff --git a/tests/test_BlockSpecial.cc b/tests/test_BlockSpecial.cc
index 8b46019c..f4ee9b6c 100644
--- a/tests/test_BlockSpecial.cc
+++ b/tests/test_BlockSpecial.cc
@@ -138,6 +138,7 @@ static std::string get_link_name()
                return "";
        }
 
+       std::string name;
        bool found = false;
        struct dirent * dentry;
        // Silence GCC [-Wparentheses] warning with double parentheses
@@ -146,6 +147,7 @@ static std::string get_link_name()
                if ( strcmp( dentry->d_name, "." )  != 0 &&
                     strcmp( dentry->d_name, ".." ) != 0    )
                {
+                       name = dentry->d_name;
                        found = true;
                        break;
                }
@@ -153,7 +155,7 @@ static std::string get_link_name()
        closedir( dir );
 
        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'";
        return "";


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]