Subject: Re: mkdir with trailing / (patch proposed)
To: Bill Studenmund <wrstuden@netbsd.org>
From: Matthew Orgass <darkstar@pgh.net>
List: tech-kern
Date: 04/30/2002 12:40:20
On Mon, 29 Apr 2002, Bill Studenmund wrote:
> On Tue, 30 Apr 2002, enami tsugutomo wrote:
> > But this is also necessary for mkdir(2) case.  Currently, fs code
> > assumes that the last components is NUL terminated (but not always,
> > since lookup code works with trailing /).  I think just always trust
> > the length of component instead is consistient.  Of course, this
> > changes protocol between each fs and fs independent layer (but is it
> > well defined?)
>
> Is it? I agree that the memcpy code is exactly the memcpy code you'd use
> for a NUL terminated string. But for example ufs only looks at cn_namelen
> characters. It copies cn_namelen + 1, but only ever looks at cn_namelen of
> them. While adding a trailing '\0' might make dumps look nice, I don't
> think we need it.

  It actually copies what it assumes to be '\0' to the disk, causing fun
corruption (and a quick panic) if that assumption isn't correct.  I
actually had trailing slash patches about two years ago but noticed the
corruption and unfortunately got sidetracked before finding the cause or
filing a PR.

> > Ya, I just didn't notice the MODMASK.  BTW, as commented, my choise
> > CREATEDIR is intended that ``we are creating new directory entry and
> > it is a directory''.
>
> Is there ever anything else we would need this for? The reason I didn't
> use "CREATE" in the name was the error test doesn't have CREATE in it
> either. It just seems a bit weird for RENAME to be adding CREATEDIR. :-)

  It is also needed for compatibility with systems that allow trailing
slashes on all files.

  The following patches should now be a complete fix to this problem.
Trailing slashes are always allowed on Linux and SYSV; I don't know if any
others need it (ibcs2?).

Matthew Orgass
darkstar@pgh.net

Index: sys/proc.h
===================================================================
RCS file: /cvsroot/syssrc/sys/sys/proc.h,v
retrieving revision 1.136
diff -u -r1.136 proc.h
--- proc.h	2002/01/11 21:16:27	1.136
+++ proc.h	2002/04/30 15:43:29
@@ -91,9 +91,9 @@
 struct emul {
 	const char	*e_name;	/* Symbolic name */
 	const char	*e_path;	/* Extra emulation path (NULL if none)*/
-#ifndef __HAVE_MINIMAL_EMUL
 	int		e_flags;	/* Miscellaneous flags, see above */
 					/* Syscall handling function */
+#ifndef __HAVE_MINIMAL_EMUL
 	const int	*e_errno;	/* Errno array */
 	int		e_nosys;	/* Offset of the nosys() syscall */
 	int		e_nsysent;	/* Number of system call entries */
@@ -127,6 +127,7 @@
  * Emulation miscelaneous flags
  */
 #define	EMUL_HAS_SYS___syscall	0x001	/* Has SYS___syscall */
+#define	EMUL_TSLASHOK		0x002	/* Trailing slashes always valid */

 /*
  * Description of a process.
Index: kern/vfs_lookup.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/vfs_lookup.c,v
retrieving revision 1.39
diff -u -r1.39 vfs_lookup.c
--- vfs_lookup.c	2001/12/08 04:09:59	1.39
+++ vfs_lookup.c	2002/04/30 15:43:30
@@ -107,6 +107,10 @@
 #endif
 	cwdi = cnp->cn_proc->p_cwdi;

+	/* Some systems allow trailing slashes on all files */
+	if (cnp->cn_proc->p_emul->e_flags & EMUL_TSLASHOK)
+		cnp->cn_flags |= TSLASHOK;
+
 	/*
 	 * Get a buffer for the name to be translated, and copy the
 	 * name into the buffer.
@@ -420,6 +424,10 @@
 		else
 			cnp->cn_flags &= ~MAKEENTRY;
 		cnp->cn_flags |= ISLASTCN;
+
+		/* trailing slashes ok for dir creation or sysv emulation */
+		if (cnp->cn_flags & TSLASHOK)
+			cnp->cn_flags &= ~REQUIREDIR;
 	} else {
 		cnp->cn_flags |= MAKEENTRY;
 		cnp->cn_flags &= ~ISLASTCN;
Index: kern/vfs_syscalls.c
===================================================================
RCS file: /cvsroot/syssrc/sys/kern/vfs_syscalls.c,v
retrieving revision 1.173
diff -u -r1.173 vfs_syscalls.c
--- vfs_syscalls.c	2001/11/12 15:25:41	1.173
+++ vfs_syscalls.c	2002/04/30 15:43:39
@@ -2825,8 +2825,8 @@
 	if ((error = namei(&fromnd)) != 0)
 		return (error);
 	fvp = fromnd.ni_vp;
-	NDINIT(&tond, RENAME, LOCKPARENT | LOCKLEAF | NOCACHE | SAVESTART,
-	    UIO_USERSPACE, to, p);
+	NDINIT(&tond, RENAME, LOCKPARENT | LOCKLEAF | NOCACHE | SAVESTART |
+	    (fvp->v_type == VDIR ? TSLASHOK : 0), UIO_USERSPACE, to, p);
 	if ((error = namei(&tond)) != 0) {
 		VOP_ABORTOP(fromnd.ni_dvp, &fromnd.ni_cnd);
 		vrele(fromnd.ni_dvp);
@@ -2913,7 +2913,8 @@
 	int error;
 	struct nameidata nd;

-	NDINIT(&nd, CREATE, LOCKPARENT, UIO_USERSPACE, SCARG(uap, path), p);
+	NDINIT(&nd, CREATE, LOCKPARENT | TSLASHOK, UIO_USERSPACE,
+	    SCARG(uap, path), p);
 	if ((error = namei(&nd)) != 0)
 		return (error);
 	vp = nd.ni_vp;
cvs server: I know nothing about ufs/ufs_lookup.c
cvs server: I know nothing about ufs/ufs_vnops.c
Index: ufs/ext2fs/ext2fs_lookup.c
===================================================================
RCS file: /cvsroot/syssrc/sys/ufs/ext2fs/ext2fs_lookup.c,v
retrieving revision 1.18
diff -u -r1.18 ext2fs_lookup.c
--- ext2fs_lookup.c	2001/11/08 02:39:07	1.18
+++ ext2fs_lookup.c	2002/04/30 15:43:41
@@ -778,7 +778,8 @@
 	} else {
 		newdir.e2d_type = 0;
 	};
-	memcpy(newdir.e2d_name, cnp->cn_nameptr, (unsigned)cnp->cn_namelen + 1);
+	memcpy(newdir.e2d_name, cnp->cn_nameptr, (unsigned)cnp->cn_namelen);
+	newdir.e2d_name[cnp->cn_namelen] = 0;
 	newentrysize = EXT2FS_DIRSIZ(cnp->cn_namelen);
 	if (dp->i_count == 0) {
 		/*
Index: compat/aout/aout_exec.c
===================================================================
RCS file: /cvsroot/syssrc/sys/compat/aout/aout_exec.c,v
retrieving revision 1.13
diff -u -r1.13 aout_exec.c
--- aout_exec.c	2001/11/13 02:07:52	1.13
+++ aout_exec.c	2002/04/30 15:43:42
@@ -65,8 +65,8 @@
 struct emul emul_netbsd_aout = {
 	"netbsd",
 	"/emul/aout",
-#ifndef __HAVE_MINIMAL_EMUL
 	EMUL_HAS_SYS___syscall,
+#ifndef __HAVE_MINIMAL_EMUL
 	NULL,
 	AOUT_SYS_syscall,
 	AOUT_SYS_MAXSYSCALL,
Index: compat/freebsd/freebsd_exec.c
===================================================================
RCS file: /cvsroot/syssrc/sys/compat/freebsd/freebsd_exec.c,v
retrieving revision 1.17
diff -u -r1.17 freebsd_exec.c
--- freebsd_exec.c	2001/11/13 02:08:06	1.17
+++ freebsd_exec.c	2002/04/30 15:43:42
@@ -53,8 +53,8 @@
 const struct emul emul_freebsd = {
 	"freebsd",
 	"/emul/freebsd",
-#ifndef __HAVE_MINIMAL_EMUL
 	EMUL_HAS_SYS___syscall,
+#ifndef __HAVE_MINIMAL_EMUL
 	NULL,
 	FREEBSD_SYS_syscall,
 	FREEBSD_SYS_MAXSYSCALL,
Index: compat/ibcs2/ibcs2_exec.c
===================================================================
RCS file: /cvsroot/syssrc/sys/compat/ibcs2/ibcs2_exec.c,v
retrieving revision 1.49
diff -u -r1.49 ibcs2_exec.c
--- ibcs2_exec.c	2001/11/13 02:08:22	1.49
+++ ibcs2_exec.c	2002/04/30 15:43:42
@@ -72,8 +72,8 @@
 const struct emul emul_ibcs2 = {
 	"ibcs2",
 	"/emul/ibcs2",
-#ifndef __HAVE_MINIMAL_EMUL
 	0,
+#ifndef __HAVE_MINIMAL_EMUL
 	native_to_ibcs2_errno,
 	IBCS2_SYS_syscall,
 	IBCS2_SYS_MAXSYSCALL,
Index: compat/irix/irix_exec.c
===================================================================
RCS file: /cvsroot/syssrc/sys/compat/irix/irix_exec.c,v
retrieving revision 1.12
diff -u -r1.12 irix_exec.c
--- irix_exec.c	2002/02/21 21:53:00	1.12
+++ irix_exec.c	2002/04/30 15:43:43
@@ -75,8 +75,8 @@
 const struct emul emul_irix_o32 = {
 	"irix o32",
 	"/emul/irix",
-#ifndef __HAVE_MINIMAL_EMUL
 	0,
+#ifndef __HAVE_MINIMAL_EMUL
 	native_to_irix_errno,
 	IRIX_SYS_syscall,
 	IRIX_SYS_MAXSYSCALL,
Index: compat/linux/common/linux_exec.c
===================================================================
RCS file: /cvsroot/syssrc/sys/compat/linux/common/linux_exec.c,v
retrieving revision 1.57
diff -u -r1.57 linux_exec.c
--- linux_exec.c	2002/03/16 20:43:53	1.57
+++ linux_exec.c	2002/04/30 15:43:43
@@ -115,8 +115,8 @@
 const struct emul emul_linux = {
 	"linux",
 	"/emul/linux",
+	EMUL_TSLASHOK,
 #ifndef __HAVE_MINIMAL_EMUL
-	0,
 	(int*)native_to_linux_errno,
 	LINUX_SYS_syscall,
 	LINUX_SYS_MAXSYSCALL,
Index: compat/mach/mach_exec.c
===================================================================
RCS file: /cvsroot/syssrc/sys/compat/mach/mach_exec.c,v
retrieving revision 1.5
diff -u -r1.5 mach_exec.c
--- mach_exec.c	2001/11/13 02:09:02	1.5
+++ mach_exec.c	2002/04/30 15:43:43
@@ -64,8 +64,8 @@
 const struct emul emul_mach = {
 	"mach",
 	"/emul/mach",
-#ifndef __HAVE_MINIMAL_EMUL
 	0,
+#ifndef __HAVE_MINIMAL_EMUL
 	0,
 	SYS_syscall,
 	SYS_MAXSYSCALL,
Index: compat/netbsd32/netbsd32_netbsd.c
===================================================================
RCS file: /cvsroot/syssrc/sys/compat/netbsd32/netbsd32_netbsd.c,v
retrieving revision 1.63
diff -u -r1.63 netbsd32_netbsd.c
--- netbsd32_netbsd.c	2002/03/16 20:43:54	1.63
+++ netbsd32_netbsd.c	2002/04/30 15:43:47
@@ -107,8 +107,8 @@
 const struct emul emul_netbsd32 = {
 	"netbsd32",
 	"/emul/netbsd32",
-#ifndef __HAVE_MINIMAL_EMUL
 	0,
+#ifndef __HAVE_MINIMAL_EMUL
 	NULL,
 	netbsd32_SYS_syscall,
 	netbsd32_SYS_MAXSYSCALL,
Index: compat/osf1/osf1_exec.c
===================================================================
RCS file: /cvsroot/syssrc/sys/compat/osf1/osf1_exec.c,v
retrieving revision 1.28
diff -u -r1.28 osf1_exec.c
--- osf1_exec.c	2001/11/13 02:09:12	1.28
+++ osf1_exec.c	2002/04/30 15:43:47
@@ -55,8 +55,8 @@
 const struct emul emul_osf1 = {
 	"osf1",
 	"/emul/osf1",
-#ifndef __HAVE_MINIMAL_EMUL
 	0,
+#ifndef __HAVE_MINIMAL_EMUL
 	(int *)osf1_errno_rxlist,
 	OSF1_SYS_syscall,
 	OSF1_SYS_MAXSYSCALL,
Index: compat/pecoff/pecoff_emul.c
===================================================================
RCS file: /cvsroot/syssrc/sys/compat/pecoff/pecoff_emul.c,v
retrieving revision 1.1
diff -u -r1.1 pecoff_emul.c
--- pecoff_emul.c	2002/03/25 06:44:46	1.1
+++ pecoff_emul.c	2002/04/30 15:43:47
@@ -74,8 +74,8 @@
 const struct emul emul_pecoff = {
 	"pecoff",
 	"/emul/pecoff",
-#ifndef __HAVE_MINIMAL_EMUL
 	EMUL_HAS_SYS___syscall,
+#ifndef __HAVE_MINIMAL_EMUL
 	0,
 	PECOFF_SYS_syscall,
 	PECOFF_SYS_MAXSYSCALL,
Index: compat/svr4/svr4_exec.c
===================================================================
RCS file: /cvsroot/syssrc/sys/compat/svr4/svr4_exec.c,v
retrieving revision 1.47
diff -u -r1.47 svr4_exec.c
--- svr4_exec.c	2001/11/13 02:09:21	1.47
+++ svr4_exec.c	2002/04/30 15:43:47
@@ -61,8 +61,8 @@
 const struct emul emul_svr4 = {
 	"svr4",
 	"/emul/svr4",
+	EMUL_TSLASHOK,
 #ifndef __HAVE_MINIMAL_EMUL
-	0,
 	native_to_svr4_errno,
 	SVR4_SYS_syscall,
 	SVR4_SYS_MAXSYSCALL,
Index: compat/svr4_32/svr4_32_exec.c
===================================================================
RCS file: /cvsroot/syssrc/sys/compat/svr4_32/svr4_32_exec.c,v
retrieving revision 1.7
diff -u -r1.7 svr4_32_exec.c
--- svr4_32_exec.c	2001/11/13 02:09:27	1.7
+++ svr4_32_exec.c	2002/04/30 15:43:48
@@ -63,8 +63,8 @@
 const struct emul emul_svr4_32 = {
 	"svr4_32",
 	"/emul/svr4_32",
+	EMUL_TSLASHOK,
 #ifndef __HAVE_MINIMAL_EMUL
-	0,
 	native_to_svr4_errno,
 	SVR4_32_SYS_syscall,
 	SVR4_32_SYS_MAXSYSCALL,
Index: compat/ultrix/ultrix_misc.c
===================================================================
RCS file: /cvsroot/syssrc/sys/compat/ultrix/ultrix_misc.c,v
retrieving revision 1.76
diff -u -r1.76 ultrix_misc.c
--- ultrix_misc.c	2002/03/16 23:55:57	1.76
+++ ultrix_misc.c	2002/04/30 15:43:49
@@ -163,8 +163,8 @@
 const struct emul emul_ultrix = {
 	"ultrix",
 	"/emul/ultrix",
-#ifndef __HAVE_MINIMAL_EMUL
 	0,
+#ifndef __HAVE_MINIMAL_EMUL
 	NULL,
 	ULTRIX_SYS_syscall,
 	ULTRIX_SYS_MAXSYSCALL,
Index: sys/namei.h
===================================================================
RCS file: /cvsroot/syssrc/sys/sys/namei.h,v
retrieving revision 1.25
diff -u -r1.25 namei.h
--- namei.h	2001/12/08 04:10:00	1.25
+++ namei.h	2002/04/30 15:52:08
@@ -111,6 +111,7 @@
 #define	NOCACHE		0x0020	/* name must not be left in cache */
 #define	FOLLOW		0x0040	/* follow symbolic links */
 #define	NOFOLLOW	0x0000	/* do not follow symbolic links (pseudo) */
+#define	TSLASHOK	0x0080	/* trailing slash allowed on non-dir */
 #define	MODMASK		0x00fc	/* mask of operational modifiers */
 /*
  * Namei parameter descriptors.
Index: ufs/ufs/ufs_lookup.c
===================================================================
RCS file: /cvsroot/syssrc/sys/ufs/ufs/ufs_lookup.c,v
retrieving revision 1.38
diff -u -r1.38 ufs_lookup.c
--- ufs_lookup.c	2001/12/18 10:57:23	1.38
+++ ufs_lookup.c	2002/04/30 16:23:08
@@ -697,7 +697,8 @@
 #endif
 	newdirp->d_ino = ip->i_number;
 	newdirp->d_namlen = cnp->cn_namelen;
-	memcpy(newdirp->d_name, cnp->cn_nameptr, (unsigned)cnp->cn_namelen + 1);
+	memcpy(newdirp->d_name, cnp->cn_nameptr, (unsigned)cnp->cn_namelen);
+	newdirp->d_name[cnp->cn_namelen] = 0;
 	if (ITOV(ip)->v_mount->mnt_maxsymlinklen > 0)
 		newdirp->d_type = IFTODT(ip->i_ffs_mode);
 	else {
Index: ufs/ufs/ufs_vnops.c
===================================================================
RCS file: /cvsroot/syssrc/sys/ufs/ufs/ufs_vnops.c,v
retrieving revision 1.85
diff -u -r1.85 ufs_vnops.c
--- ufs_vnops.c	2001/12/23 16:16:59	1.85
+++ ufs_vnops.c	2002/04/30 16:23:12
@@ -729,7 +729,8 @@
 		newdir.d_ino = WINO;
 		newdir.d_namlen = cnp->cn_namelen;
 		memcpy(newdir.d_name, cnp->cn_nameptr,
-		    (unsigned)cnp->cn_namelen + 1);
+		    (unsigned)cnp->cn_namelen);
+		newdir.d_name[cnp->cn_namelen] = 0;
 		newdir.d_type = DT_WHT;
 		error = ufs_direnter(dvp, NULL, &newdir, cnp, NULL);
 		break;