NetBSD Problem Report #10237

Received: (qmail 13624 invoked from network); 30 May 2000 19:40:52 -0000
Message-Id: <20000530194045.AE5D9DD@proven.weird.com>
Date: Tue, 30 May 2000 15:40:45 -0400 (EDT)
From: woods@weird.com (Greg A. Woods)
Sender: woods@proven.weird.com
Reply-To: woods@planix.com (Greg A. Woods)
To: gnats-bugs@gnats.netbsd.org
Subject: nfs/nfs_vnops.c:nfs_sillyrename() has extremely silly limitations
X-Send-Pr-Version: 3.95

>Number:         10237
>Category:       kern
>Synopsis:       nfs/nfs_vnops.c:nfs_sillyrename() has extremely silly limitations
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue May 30 19:41:00 +0000 2000
>Closed-Date:    
>Last-Modified:  
>Originator:     Greg A. Woods & Andreas Wrede
>Release:        Mon Mar 20 09:47:15 EST 2000
>Organization:
Planix, Inc.; Toronto, Ontario; Canada
>Environment:

System: NetBSD 1.4V i386

>Description:

	The nfs_sillyrename() function in nfs/nfs_vnops.c is extrmely
	silly, and prevents "rm" from removing more than 58 files in
	any given directory with one invocation due to the built-in
	limitations in the algorithm used to avoid name collisions.

	In my book it's a lot better to avoid creating the
	circumstance for name collisions in the first place, rather
	than try to deal with them after the fact.

	This bug would be very hard to trip over were it not for the
	NFS/nullfs bug already reported in PR#10235 making it almost
	instantly visible.

>How-To-Repeat:

	# create a scenario with a "null" filesystem mount looped back
	# over an NFS filesystem and find a "large" junk hierarchy to
	# remove.  (see PR#10235 for more info)  This scenario ensures
	# that all files removed will have to be renamed to .nfs*
	# "silly" names.

	$ cd /var/spool/ftp/pub/mirror/vm/snapshots
	$ rm -rf snapshot-19990627
	rm: snapshot-19990627/include: Directory not empty
	rm: snapshot-19990627/bin: Directory not empty
	rm: snapshot-19990627/lib: Directory not empty
	rm: snapshot-19990627/global/mail_addr.c: Invalid argument
	rm: snapshot-19990627/global/is_header.h: Invalid argument
	rm: snapshot-19990627/global/sent.h: Invalid argument
	rm: snapshot-19990627/global/is_header.c: Invalid argument
	rm: snapshot-19990627/global/namadr_list.c: Invalid argument
	rm: snapshot-19990627/global/opened.c: Invalid argument
	rm: snapshot-19990627/global/opened.h: Invalid argument
	rm: snapshot-19990627/global/rec2stream.c: Invalid argument
	rm: snapshot-19990627/global/mail_addr_find.h: Invalid argument
	rm: snapshot-19990627/global/debug_process.h: Invalid argument
	rm: snapshot-19990627/global/recdump.c: Invalid argument
	rm: snapshot-19990627/global/debug_process.c: Invalid argument
	^?

>Fix:

	apply the following patch (noting that a couple of sprintf()s
	are going to be one heck of a lot faster than up to 57
	nfs_lookitup() calls per unlink!):

Index: nfs/nfs_vnops.c
===================================================================
RCS file: /cvs/NetBSD/src/sys/nfs/nfs_vnops.c,v
retrieving revision 1.1.1.5
diff -c -r1.1.1.5 nfs_vnops.c
*** nfs/nfs_vnops.c	2000/03/21 01:02:24	1.1.1.5
--- nfs/nfs_vnops.c	2000/05/30 17:42:23
***************
*** 2434,2440 ****
  		vrele(newvp);
  	return (error);
  }
- static char hextoasc[] = "0123456789abcdef";

  /*
   * Silly rename. To make the NFS filesystem that is stateless look a little
--- 2434,2439 ----
***************
*** 2466,2489 ****
  	sp->s_dvp = dvp;
  	VREF(dvp);

! 	/* Fudge together a funny name */
  	pid = cnp->cn_proc->p_pid;
! 	memcpy(sp->s_name, ".nfsAxxxx4.4", 13);
! 	sp->s_namlen = 12;
! 	sp->s_name[8] = hextoasc[pid & 0xf];
! 	sp->s_name[7] = hextoasc[(pid >> 4) & 0xf];
! 	sp->s_name[6] = hextoasc[(pid >> 8) & 0xf];
! 	sp->s_name[5] = hextoasc[(pid >> 12) & 0xf];

- 	/* Try lookitups until we get one that isn't there */
- 	while (nfs_lookitup(dvp, sp->s_name, sp->s_namlen, sp->s_cred,
- 		cnp->cn_proc, (struct nfsnode **)0) == 0) {
- 		sp->s_name[4]++;
- 		if (sp->s_name[4] > 'z') {
- 			error = EINVAL;
- 			goto bad;
- 		}
- 	}
  	error = nfs_renameit(dvp, cnp, sp);
  	if (error)
  		goto bad;
--- 2465,2478 ----
  	sp->s_dvp = dvp;
  	VREF(dvp);

! 	/* Fudge together a funny, but unique, name */
  	pid = cnp->cn_proc->p_pid;
! 	memcpy(sp->s_name, NFS_SILLYNAME, sizeof(NFS_SILLYNAME));
! 	sp->s_namlen = sizeof(NFS_SILLYNAME) - 1;
! 	/* if this isn't guaranteed unique then add in the va_fsid too */
! 	sprintf(&sp->s_name[NFS_SILLYNAME_INO_OFF], "%08lx", np->n_vattr->va_fileid);
! 	sprintf(&sp->s_name[NFS_SILLYNAME_PID_OFF], "%04x", pid);

  	error = nfs_renameit(dvp, cnp, sp);
  	if (error)
  		goto bad;
Index: nfs/nfsnode.h
===================================================================
RCS file: /cvs/NetBSD/src/sys/nfs/nfsnode.h,v
retrieving revision 1.1.1.5
diff -c -r1.1.1.5 nfsnode.h
*** nfs/nfsnode.h	2000/03/21 01:02:26	1.1.1.5
--- nfs/nfsnode.h	2000/05/30 17:39:37
***************
*** 47,52 ****
--- 47,62 ----
  #endif

  /*
+  * Filename pattern for sillyrename.s_name
+  *
+  * Warning: don't change without fixing up the code which munges it!
+  * (in nfs/nfs_vnops.c)
+  */
+ #define NFS_SILLYNAME		".nfs.xxxxxxxx.xxxx"
+ #define NFS_SILLYNAME_INO_OFF	5
+ #define NFS_SILLYNAME_PID_OFF	14
+ 
+ /*
   * Silly rename structure that hangs off the nfsnode until the name
   * can be removed by nfs_inactive()
   */
***************
*** 54,60 ****
  	struct	ucred *s_cred;
  	struct	vnode *s_dvp;
  	long	s_namlen;
! 	char	s_name[20];
  };

  /*
--- 64,70 ----
  	struct	ucred *s_cred;
  	struct	vnode *s_dvp;
  	long	s_namlen;
! 	char	s_name[20];		/* XXX sizeof(NFS_SILLYNAME)+1 ? */
  };

  /*
>Release-Note:
>Audit-Trail:
>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.