lib/37730: libc-level getdirentries compat behavior loses

[dholland%eecs.harvard.edu@localhost]

>Number:         37730
>Category:       lib
>Synopsis:       libc-level getdirentries compat behavior loses
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    lib-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Jan 09 18:45:01 +0000 2008
>Originator:     David A. Holland <dholland%eecs.harvard.edu@localhost>
>Release:        NetBSD 4.99.31 (20071123)
>Organization:
unorganized
>Environment:
System: NetBSD tanaqui 4.99.31 NetBSD 4.99.31 (TANAQUI) #19: Tue Sep 11 
19:46:35 EDT 2007 dholland@tanaqui:/usr/src/sys/arch/i386/compile/TANAQUI i386
Architecture: i386
Machine: i386
>Description:

If you compile new code that uses getdirentries() instead of
getdents(), it gets dirent12 structs returned but necessarily ends up
using the current struct dirent to examine them. Needless to say, this
doesn't work.

This turned out to be was one of the things breaking Wine (although
Wine has just been fixed) so it's not a completely vacuous issue, even
though sane code should be using readdir().

>How-To-Repeat:

Here's a test program that creates a directory and examines its '.'
entry.

   ------

#include <sys/types.h>
#include <sys/fcntl.h>
#include <unistd.h>
#include <dirent.h>
#include <err.h>

/* from sys/compat/sys/dirent.h */
struct dirent12 {
        u_int32_t d_fileno;             /* file number of entry */
        u_int16_t d_reclen;             /* length of this record */
        u_int8_t  d_type;               /* file type, see below */
        u_int8_t  d_namlen;             /* length of string in d_name */
        char    d_name[255 + 1];        /* name must be no longer than this */
};

static void init(void) {
   if (mkdir("testdir", 0775) < 0) {
      err(1, "mkdir: testdir");
   }
   if (chdir("testdir") < 0) {
      err(1, "chdir: testdir");
   }
}

static void cleanup(void) {
   if (chdir("..") < 0) {
      err(1, "chdir: ..");
   }
   if (rmdir("testdir") < 0) {
      err(1, "rmdir: testdir");
   }
}

static int test(void) {
   int fd;
   int result;
   long pos;
   struct stat st;
   union {
      char buf[8192];
      struct dirent d;
      struct dirent12 d12;
   } x;

   fd = open(".", O_RDONLY);
   if (fd<0) {
      warn(".");
      return 1;
   }

   if (fstat(fd, &st) < 0) {
      warn(".: fstat");
      close(fd);
      return 1;
   }

   result = getdirentries(fd, x.buf, sizeof(x.buf), &pos);
   if (result < 0) {
      warn(".: getdirentries");
      close(fd);
      return 1;
   }

   if (x.d.d_fileno == st.st_ino) {
      warnx("struct direct inode matches");
   }
   if (x.d12.d_fileno == st.st_ino) {
      warnx("struct direct12 inode matches");
   }

   if (x.d.d_namlen == 1) {
      warnx("struct direct namlen matches");
   }
   if (x.d12.d_namlen == 1) {
      warnx("struct direct12 namlen matches");
   }

   if (x.d.d_name[0]=='.' && x.d.d_name[1]==0) {
      warnx("struct direct name matches");
   }
   if (x.d12.d_name[0]=='.' && x.d12.d_name[1]==0) {
      warnx("struct direct12 name matches");
   }

   close(fd);
   return 0;
}

int main() {
   int result;

   init();
   result = test();
   cleanup();

   return result;
}

   ------

>Fix:
Mumble.

getdirentries() is not actually declared in /usr/include, but this
doesn't stop programs from linking to it.

Removing the getdirentries entry point from libc would break compat,
so that's out.

Perhaps the best approach would be to declare it and add
__RENAME(__getdirentries___is_not_supported);

That way newly compiled code would at least fail to link.

Alternatively, one could add a real compat stub to libc (which is all
of two lines of code...) but bringing it back from the dead isn't
exactly desirable.




>Unformatted: