Opened 7 years ago

Closed 4 years ago

#9466 closed bug (invalid)

Bug in readdir_r?

Reported by: fabbo Owned by: nobody
Priority: normal Milestone: R1
Component: System/POSIX Version: R1/alpha4.1
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: no Platform: All

Description

I am using Haiku to build the following program

#include <stdlib.h>
#include <stdio.h>

#include <errno.h>
#include <dirent.h>

int main(void) {
  DIR *plugindir = opendir("loaders");
  struct dirent direntry;
  struct dirent *dirresult;
  int i;

  if (plugindir == NULL) {
    return;
  }
  do {
#ifdef CRASH
    int res = readdir_r(plugindir, &direntry, &dirresult);
    if (res != 0) {
     printf("fail %s\n",strerror(res));
      break;
    }
#else
  dirresult = readdir(plugindir);
#endif

    if (dirresult == NULL)
      break;

    printf("file %s\n",dirresult->d_name);
  } while (1);
  closedir(plugindir);
  return 0;
}

I use native gcc 2.95.3 (that is, no cross-compiling, gcc runs on Haiku itself) to build it (by the way, are newer version of gcc available?). If I compile it with -DCRASH, this is the output.

file .
fail Operation not supported
cr: /storage/haiku-releases/r1alpha4/haiku/src/system/libroot/posix/malloc/processheap.cpp:166:void BPrivate::processHeap::free(void *): sb
Kill Thread

Change History (6)

comment:1 by fabbo, 7 years ago

The same program runs fine on Linux, with and without -DCRASH

comment:2 by hamish, 7 years ago

Resolution: invalid
Status: newclosed

You are expected to allocate a dirent structure big enough for the names of the files in the directory. The d_name array in the struct definition only has space for one character. In this case, readdir_r scribbles over plugindir, causing the second call to readdir_r to fail and the call to closedir to crash.

The reason it works on Linux is because struct dirent has a larger d_name. But since POSIX doesn't specify the length of the array, portable code shouldn't rely on that.

comment:3 by nielx, 4 years ago

Resolution: invalid
Status: closedreopened

I would like to reopen discussion on this issue, as I have recently been bitten by it as well. On all major platforms (Linux, *BSD, MacOS X) the definition of the dirent_t struct contains a name member that is the size of the max path plus one. It is not unlikely that other ported apps also depend on this behavior, and that there actually already are bugs in existing packages because of this.

comment:4 by nielx, 4 years ago

Component: - GeneralSystem/POSIX

comment:5 by bonefish, 4 years ago

At least the Linux man page is pretty explicit about how to use readdir_r():

Since POSIX.1 does not specify the size of the d_name field, and other nonstandard fields may precede that field within the dirent structure, portable applications that use readdir_r() should allocate the buffer whose address is passed in entry as follows:

name_max = pathconf(dirpath, _PC_NAME_MAX);
if (name_max == -1)         /* Limit not defined, or error */
    name_max = 255;         /* Take a guess */
len = offsetof(struct dirent, d_name) + name_max + 1;
entryp = malloc(len);

That aside, it is not entirely trivial to change the size of the d_name field. There's quite a bit of code that would need to be reviewed (file systems, VFS, storage kit) and the (binary/source) compatibility implications need to be weighed as well.

comment:6 by nielx, 4 years ago

Resolution: invalid
Status: reopenedclosed

Thanks for the backstory. I will upstream a patch that will follow that example.

Note: See TracTickets for help on using tickets.