NetBSD Problem Report #37730

From dholland@eecs.harvard.edu  Wed Jan  9 18:43:17 2008
Return-Path: <dholland@eecs.harvard.edu>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id E98E263BD87
	for <gnats-bugs@gnats.NetBSD.org>; Wed,  9 Jan 2008 18:43:16 +0000 (UTC)
Message-Id: <20080109184332.05D4BF8B9@tanaqui.eecs.harvard.edu>
Date: Wed,  9 Jan 2008 13:43:31 -0500 (EST)
From: dholland@eecs.harvard.edu
Reply-To: dholland@eecs.harvard.edu
To: gnats-bugs@NetBSD.org
Subject: libc-level getdirentries compat behavior loses
X-Send-Pr-Version: 3.95

>Number:         37730
>Category:       lib
>Synopsis:       libc-level getdirentries compat behavior loses
>Confidential:   no
>Severity:       non-critical
>Priority:       low
>Responsible:    lib-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Wed Jan 09 18:45:01 +0000 2008
>Closed-Date:    Sun Feb 22 06:35:41 +0000 2009
>Last-Modified:  Sun Feb 22 06:35:41 +0000 2009
>Originator:     David A. Holland <dholland@eecs.harvard.edu>
>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.




>Release-Note:

>Audit-Trail:
From: David Holland <dholland-bugs@netbsd.org>
To: gnats-bugs@NetBSD.org
Cc: lib-bug-people@netbsd.org, gnats-admin@netbsd.org,
	netbsd-bugs@netbsd.org
Subject: Re: lib/37730: libc-level getdirentries compat behavior loses
Date: Sun, 24 Aug 2008 23:54:30 +0000

 On Wed, Jan 09, 2008 at 06:45:01PM +0000, dholland@eecs.harvard.edu wrote:
  > 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.
  > 
  > [...]
  >
  > >Fix:
  > Mumble.

 I discussed this with Christos some time ago. The conclusion was that
 (1) getdirentries() can and should be tagged with __warn_references,
 and (2) nothing more aggressive is workable.

 So here, finally, is the patch:

 Index: compat_getdirentries.c
 ===================================================================
 RCS file: /cvsroot/src/lib/libc/compat/sys/compat_getdirentries.c,v
 retrieving revision 1.1
 diff -u -r1.1 compat_getdirentries.c
 --- compat_getdirentries.c	13 Sep 2005 01:44:09 -0000	1.1
 +++ compat_getdirentries.c	24 Aug 2008 23:48:40 -0000
 @@ -44,6 +44,9 @@
  #include <compat/include/dirent.h>
  #include <unistd.h>

 +__warn_references(getdirentries,
 +    "reference to compatibility-only getdirentries(); this will break; use getdents() or readdir() instead")
 +
  int
  getdirentries(fd, buf, nbytes, basep)
  	int fd, nbytes;


 -- 
 David A. Holland
 dholland@netbsd.org

From: "David A. Holland" <dholland@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/37730 CVS commit: src/lib/libc/compat/sys
Date: Sun, 22 Feb 2009 06:33:38 +0000 (UTC)

 Module Name:	src
 Committed By:	dholland
 Date:		Sun Feb 22 06:33:38 UTC 2009

 Modified Files:
 	src/lib/libc/compat/sys: compat_getdirentries.c

 Log Message:
 Add __warn_references to getdirentries(), per PR 37730. Okayed by Christos
 a while back.


 To generate a diff of this commit:
 cvs rdiff -r1.1 -r1.2 src/lib/libc/compat/sys/compat_getdirentries.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->closed
State-Changed-By: dholland@NetBSD.org
State-Changed-When: Sun, 22 Feb 2009 06:35:41 +0000
State-Changed-Why:
fixed


>Unformatted:

Home	PR Database Search