4

Posting snippets of my code here. I am trying to get a footing around debugging. 

struct dirent *s_dirent;
char path[300];
....
bzero(path,300);
...
fd_dir = opendir(path);
while((s_dirent = readdir(fd_dir))!=NULL)
{
     if(s_dirent->d_name[0] == '.')
          continue;
     else
          break;
 }
if(s_dirent == NULL)
{
   if(closedir(fd_dir)!=0)
       perror("Error on closedir");
 }
else
{

  if(closedir(fd_dir)!=0)/*Line number 249*/
      perror("Error on closedir");

  /*some comments*/
  strcat(path,"/");
  strcat(path,s_dirent->d_name);/*Line number 254*/
 }

Valgrind output:

==3287== Invalid read of size 1
==3287==    at 0x40069E0: strcat (mc_replace_strmem.c:176)
==3287==    by 0x804D6B4: online_bck (backup_manager.c:254)
==3287==    by 0x8049F96: on_bck_beg (TxFS_manager.c:181)
==3287==    by 0x8049818: handler (Reader.c:236)
==3287==    by 0xBF5F18: start_thread (in /lib/libpthread-2.12.90.so)
==3287==    by 0xB37A2D: clone (in /lib/libc-2.12.90.so)
==3287==  Address 0x402a39b is 35 bytes inside a block of size 32,792 free'd
==3287==    at 0x40057F6: free (vg_replace_malloc.c:325)
==3287==    by 0xAF6C67: closedir (in /lib/libc-2.12.90.so)
==3287==    by 0x804D65A: online_bck (backup_manager.c:249)
==3287==    by 0x8049F96: on_bck_beg (TxFS_manager.c:181)
==3287==    by 0x8049818: handler (Reader.c:236)
==3287==    by 0xBF5F18: start_thread (in /lib/libpthread-2.12.90.so)
==3287==    by 0xB37A2D: clone (in /lib/libc-2.12.90.so)

Any help will be highly appreciated. Thanks

osgx
  • 90,338
  • 53
  • 357
  • 513
Lipika Deka
  • 3,774
  • 6
  • 43
  • 56

3 Answers3

5

You should not access data, returned by readdir() after calling a closedir(). This is because closedir() may free any resources (e.g. memory), allocated in opendir/readdir.

If you want to save dirent* struct, you can switch to readdir_r variant of readdir (with different set of parameters).

UPDATE: Decoding of Valgrind output:

        V - note single space here; it is beginning of error message. 
==3287== Invalid read of size 1
==3287==    at 0x40069E0: strcat (mc_replace_strmem.c:176)

 backtrace skipped

Valgrind says that error is Reading of invalid data, sized 1 byte, which is not a Memory Leak. It is incorrect memory access. And the actor of this read is the strcat() (called by skipped backtrace). Why the data is invalid? There is submessage

        VV - note two spaces here, it is continuation of error message
==3287==  Address 0x402a39b is 35 bytes inside a block of size 32,792 free'd
==3287==    at 0x40057F6: free (vg_replace_malloc.c:325)
==3287==    by 0xAF6C67: closedir (in /lib/libc-2.12.90.so)

The byte is invalid (not allowed to be read from) because it is part of memory segment which was free-d (you can't read from the memory you just free-d). Whom did this? Look at backtrace: closedir was the caller of free.

osgx
  • 90,338
  • 53
  • 357
  • 513
  • 1
    or maybe copy it else where before calling closedir() – Lipika Deka Aug 06 '11 at 02:31
  • Yes, but readdir_r will place dirent to user-supplied buffer directly, without copy. I should say, that my answer is pure speculation, and neither of Linux, SUS, Solaris man pages of readdir/closedir describes your case. But Valgrind output says, that strcmp trys to access a free-d data and closedir the only who can free it. – osgx Aug 06 '11 at 02:33
  • While what you say sounds plausible, neither the readdir() or closedir() man pages mention that the static buffer pointer returned by readdir() is deallocated by closedir(). Looking at the call stack where it was freed, it does look like that is what happened: `at 0x40057F6: free (vg_replace_malloc.c:325)` called from `by 0xAF6C67: closedir (in /lib/libc-2.12.90.so)` – Stéphane Aug 06 '11 at 02:35
  • 1
    or move the closedir to after the strcat()? – Stéphane Aug 06 '11 at 02:40
  • Man pages were read by me BEFORE posting. This is a BUG of glibc, because the implementation limit was not documented. – osgx Aug 06 '11 at 02:43
  • IBM documents this: http://publib.boulder.ibm.com/infocenter/iseries/v5r3/index.jsp?topic=%2Fapis%2Fclosedir.htm " closedir() ... frees the buffer that readdir() uses when reading the directory stream." – osgx Aug 06 '11 at 02:46
  • Appreciate the detailed explanation @osgx – Lipika Deka Aug 06 '11 at 02:53
  • 2
    @Stéphane: I can't find anywhere POSIX documents that `closedir` may free the buffer returned by `readdir`, but that seems to be a defect in POSIX. It's obviously their intent that the buffer be in the `DIR` structure since a call to `readdir` for one open `DIR` is not allowed to clobber the `dirent` returned by a call to `readdir` for a different `DIR`. There is no way to meet these requirements without leaking memory unless `closedir` frees the buffer. – R.. GitHub STOP HELPING ICE Aug 06 '11 at 03:00
0

s_dirent == NULL, check the while loop exit condition.

Steve-o
  • 12,678
  • 2
  • 41
  • 60
  • @Steave-o am sorry to not have posted the check for NULL that I had done before. But I had already checked for that condition. Thanks for your interest – Lipika Deka Aug 06 '11 at 02:27
0

By the time you get to the strcat(), you've already exited the while loop. The while loop exits once s_dirent is NULL.

But as soon as s_direct is NULL, you run this:

strcat(path,"/");
strcat(path,s_dirent->d_name);/*Line number 254*/

...which attempts to dereference s_dirent. Thus, valgrind then tells you to check the call to strcat() being called from backup_manager.c:254:

==3287== Invalid read of size 1
==3287==    at 0x40069E0: strcat (mc_replace_strmem.c:176)
==3287==    by 0x804D6B4: online_bck (backup_manager.c:254)

I'm guessing you're hitting this only when you're processing an empty directory which contains entries for ".", "..", or directories that only contain hidden files which start with "."?

Stéphane
  • 19,459
  • 24
  • 95
  • 136
  • am sorry to not have posted the check for NULL that I had done before. But I had already checked for that condition and that is not leading to the error. Thanks for your interest – Lipika Deka Aug 06 '11 at 02:28