Re: namei() returns EISDIR for / (Re: svn commit: r203990 - head/lib/libc/sys)

2010-03-26 Thread Jaakko Heinonen
On 2010-03-05, Jaakko Heinonen wrote:
 I have updated the patch taking some of bde's comments into account. The
 new version also includes updates for namei(9) manual page.

Yet another update:

http://people.freebsd.org/~jh/patches/lookup-root.4.diff

I have committed the relookup() part as r205682.

Unfortunately errno translation is needed also for open(/, O_CREAT)
and undelete(/).

-- 
Jaakko
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: namei() returns EISDIR for / (Re: svn commit: r203990 - head/lib/libc/sys)

2010-03-26 Thread Alexander Best
Jaakko Heinonen schrieb am 2010-03-26:
 On 2010-03-05, Jaakko Heinonen wrote:
  I have updated the patch taking some of bde's comments into
  account. The
  new version also includes updates for namei(9) manual page.

 Yet another update:

 http://people.freebsd.org/~jh/patches/lookup-root.4.diff

 I have committed the relookup() part as r205682.

 Unfortunately errno translation is needed also for open(/, O_CREAT)
 and undelete(/).

you might want to consider posting your patch on
http://reviews.freebsdish.org. that way it'll be easier to review it.

-- 
Alexander Best
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: namei() returns EISDIR for / (Re: svn commit: r203990 - head/lib/libc/sys)

2010-03-11 Thread Jaakko Heinonen
On 2010-03-10, Alexander Best wrote:
 could this panic have been triggered by the patch?

It doesn't look like it's caused by the patch.

 panic() at panic+0x15f
 _mtx_lock_flags() at _mtx_lock_flags+0xc5
 fdesc_allocvp() at fdesc_allocvp+0xbf
 fdesc_lookup() at fdesc_lookup+0x15c
 
 this was 100% reducible when doing `portsnap fetch` though i changed a lot of
 stuff in my kernel config and reverted a lot of src patches to resolve the
 issue so i'm not sure what exactly was causing it.

The panic happened in fdescfs code. Did you have local patches related
to fdescfs?

-- 
Jaakko
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: namei() returns EISDIR for / (Re: svn commit: r203990 - head/lib/libc/sys)

2010-03-11 Thread Alexander Best
Jaakko Heinonen schrieb am 2010-03-11:
 On 2010-03-10, Alexander Best wrote:
  could this panic have been triggered by the patch?

 It doesn't look like it's caused by the patch.

  panic() at panic+0x15f
  _mtx_lock_flags() at _mtx_lock_flags+0xc5
  fdesc_allocvp() at fdesc_allocvp+0xbf
  fdesc_lookup() at fdesc_lookup+0x15c

  this was 100% reducible when doing `portsnap fetch` though i
  changed a lot of
  stuff in my kernel config and reverted a lot of src patches to
  resolve the
  issue so i'm not sure what exactly was causing it.

 The panic happened in fdescfs code. Did you have local patches
 related
 to fdescfs?

after reverting a few patches (including yours) i got rid of the problem. i
then re-applied your patch and noticed that (as you said) it wasn't causing
the panic.

i don't have any fdescfs specific patches in my src. i suspect however [1]
being responsible for the panic. after backing it out i got no more panics in
connection with `portsnap fetch`.

[1] http://www.mail-archive.com/freebsd-hackers@freebsd.org/msg70400.html

thanks for the help

oh...and btw.:

i'm not sure but i think i've asked this question once before in this thread:

in sys/kern/vfs_syscalls.c:kern_rmdirat() there's still local code to check
for . and / after applying your patch. isn't this all being done by
calling namei() now?

cheers.
alex
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: namei() returns EISDIR for / (Re: svn commit: r203990 - head/lib/libc/sys)

2010-03-11 Thread Jaakko Heinonen
On 2010-03-11, Alexander Best wrote:
 in sys/kern/vfs_syscalls.c:kern_rmdirat() there's still local code to check
 for . and / after applying your patch. isn't this all being done by
 calling namei() now?

Looking at it quickly I think that the . case is handled by lookup()
since r199137. However the VV_ROOT test is for all mount points not just
for /.

-- 
Jaakko
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: namei() returns EISDIR for / (Re: svn commit: r203990 - head/lib/libc/sys)

2010-03-10 Thread Alexander Best
Jaakko Heinonen schrieb am 2010-03-05:
 On 2010-02-28, Jaakko Heinonen wrote:
  http://people.freebsd.org/~jh/patches/lookup-root.diff

 I have updated the patch taking some of bde's comments into account.
 The
 new version also includes updates for namei(9) manual page.

could this panic have been triggered by the patch?

Fatal trap 12: page fault while in kernel mode
cpuid = 0; apic id = 00
fault virtual address   = 0x12
fault code  = supervisor read data, page not present
instruction pointer = 0x20:0x8048af0f
stack pointer   = 0x28:0xff8040495000
frame pointer   = 0x28:0xff8040495020
code segment= base 0x0, limit 0xf, type 0x1b
= DPL 0, pres 1, long 1, def32 0, gran 1
processor eflags= interrupt enabled, resume, IOPL = 0
current process = 1402 (sh)

Tracing pid 1402 tid 100092 td 0xff000729d000
strlen() at strlen+0x1f
kvprintf() at kvprintf+0x1027
vsnprintf() at vsnprintf+0x48
panic() at panic+0x15f
_mtx_lock_flags() at _mtx_lock_flags+0xc5
fdesc_allocvp() at fdesc_allocvp+0xbf
fdesc_lookup() at fdesc_lookup+0x15c
VOP_LOOKUP_APV() at VOP_LOOKUP_APV+0x11c
VOP_LOOKUP() at VOP_LOOKUP+0x45
lookup() at lookup+0x8cf
namei() at namei+0x730
vn_open_cred() at vn_open_cred+0x114
vn_open() at vn_open+0x55
kern_openat() at kern_openat+0x178
kern_open() at kern_open+0x3e
open() at open+0x26
syscall() at syscall+0x2a7
Xfast_syscall() at Xfast_syscall+0xe1
--- syscall (5, FreeBSD ELF64, open), rip = 0x8009a3bec, rsp = 0x7fffd3c8,
rbp = 0x800e143b0 ---

this was 100% reducible when doing `portsnap fetch` though i changed a lot of
stuff in my kernel config and reverted a lot of src patches to resolve the
issue so i'm not sure what exactly was causing it.

cheers.
alex


 The patch is attached to this mail and also found at:

 http://people.freebsd.org/~jh/patches/lookup-root.2.diff

___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: namei() returns EISDIR for / (Re: svn commit: r203990 - head/lib/libc/sys)

2010-03-07 Thread Jaakko Heinonen
On 2010-03-06, Garrett Cooper wrote:
 
         http://people.freebsd.org/~jh/patches/lookup-root.2.diff
 
 1. EBUSY's new definition doesn't look correct for rename(2) given
 POSIX's definition (
 http://www.opengroup.org/onlinepubs/009695399/functions/rename.html ):
 
 [EBUSY]
 [CX] [Option Start] The directory named by old or new is currently
 in use by the system or another process, and the implementation
 considers this an error. [Option End]

Well, I don't think that it conflicts with POSIX. The root directory is
in use by the system and it's not allowed to be renamed.

 2. I also did some quick snooping around and all filesystems that
 support symlinks that FreeBSD has, sans ZFS [*], seem to support
 EMLINK. Should we add this to the documentation?

Probably, but it's not related to the changes made by the patch.

-- 
Jaakko
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: namei() returns EISDIR for / (Re: svn commit: r203990 - head/lib/libc/sys)

2010-03-06 Thread Garrett Cooper
On Thu, Mar 4, 2010 at 9:57 PM, Jaakko Heinonen j...@freebsd.org wrote:
 On 2010-02-28, Jaakko Heinonen wrote:
     http://people.freebsd.org/~jh/patches/lookup-root.diff

 I have updated the patch taking some of bde's comments into account. The
 new version also includes updates for namei(9) manual page.

 The patch is attached to this mail and also found at:

        http://people.freebsd.org/~jh/patches/lookup-root.2.diff

1. EBUSY's new definition doesn't look correct for rename(2) given
POSIX's definition (
http://www.opengroup.org/onlinepubs/009695399/functions/rename.html ):

[EBUSY]
[CX] [Option Start] The directory named by old or new is currently
in use by the system or another process, and the implementation
considers this an error. [Option End]

2. I also did some quick snooping around and all filesystems that
support symlinks that FreeBSD has, sans ZFS [*], seem to support
EMLINK. Should we add this to the documentation?

[EMLINK]
[CX] [Option Start] The file named by old is a directory, and the
link count of the parent directory of new would exceed {LINK_MAX}.
[Option End]

Thanks!
-Garrett

[*] I took a really superficial look at ZFS and this is what I found
(from zfs_pathconf):

switch (cmd) {
case _PC_LINK_MAX:
*valp = INT_MAX;
return (0);
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: namei() returns EISDIR for / (Re: svn commit: r203990 - head/lib/libc/sys)

2010-03-04 Thread Jaakko Heinonen
On 2010-02-28, Jaakko Heinonen wrote:
 http://people.freebsd.org/~jh/patches/lookup-root.diff

I have updated the patch taking some of bde's comments into account. The
new version also includes updates for namei(9) manual page.

The patch is attached to this mail and also found at:

http://people.freebsd.org/~jh/patches/lookup-root.2.diff

-- 
Jaakko
Index: sys/kern/vfs_lookup.c
===
--- sys/kern/vfs_lookup.c	(revision 204620)
+++ sys/kern/vfs_lookup.c	(working copy)
@@ -565,18 +565,26 @@ dirloop:
 	}
 
 	/*
-	 * Check for degenerate name (e.g. / or )
-	 * which is a way of talking about a directory,
-	 * e.g. like /. or ..
+	 * Check for  which represents the root directory after slash
+	 * removal.
 	 */
 	if (cnp-cn_nameptr[0] == '\0') {
-		if (dp-v_type != VDIR) {
-			error = ENOTDIR;
-			goto bad;
-		}
-		if (cnp-cn_nameiop != LOOKUP) {
-			error = EISDIR;
-			goto bad;
+		KASSERT(dp-v_type == VDIR, (dp is not a directory));
+		/* 
+		 * We can't provide a parent node for CREATE,
+		 * DELETE and RENAME operations.
+		 */
+		switch (cnp-cn_nameiop) {
+		case CREATE:
+			error = EEXIST;
+			goto bad;
+		case DELETE:
+		case RENAME:
+			error = EBUSY;
+			goto bad;
+		default:
+			KASSERT(cnp-cn_nameiop == LOOKUP,
+			(nameiop must be LOOKUP));
 		}
 		if (wantparent) {
 			ndp-ni_dvp = dp;
@@ -948,19 +956,17 @@ relookup(struct vnode *dvp, struct vnode
 #endif
 
 	/*
-	 * Check for degenerate name (e.g. / or )
-	 * which is a way of talking about a directory,
-	 * e.g. like /. or ..
+	 * Check for  which represents the root directory after slash
+	 * removal.
 	 */
 	if (cnp-cn_nameptr[0] == '\0') {
-		if (cnp-cn_nameiop != LOOKUP || wantparent) {
-			error = EISDIR;
-			goto bad;
-		}
-		if (dp-v_type != VDIR) {
-			error = ENOTDIR;
-			goto bad;
-		}
+		KASSERT(dp-v_type == VDIR, (dp is not a directory));
+		/*
+		 * Support only LOOKUP for / because lookup()
+		 * can't succeed for CREATE, DELETE and RENAME.
+		 */
+		KASSERT(cnp-cn_nameiop == LOOKUP, (nameiop must be LOOKUP));
+
 		if (!(cnp-cn_flags  LOCKLEAF))
 			VOP_UNLOCK(dp, 0);
 		*vpp = dp;
Index: sys/kern/vfs_syscalls.c
===
--- sys/kern/vfs_syscalls.c	(revision 204620)
+++ sys/kern/vfs_syscalls.c	(working copy)
@@ -1841,7 +1841,7 @@ restart:
 	NDINIT_AT(nd, DELETE, LOCKPARENT | LOCKLEAF | MPSAFE | AUDITVNODE1,
 	pathseg, path, fd, td);
 	if ((error = namei(nd)) != 0)
-		return (error == EINVAL ? EPERM : error);
+		return ((error == EINVAL || error == EBUSY) ? EPERM : error);
 	vfslocked = NDHASGIANT(nd);
 	vp = nd.ni_vp;
 	if (vp-v_type == VDIR  oldinum == 0) {
@@ -3617,9 +3617,6 @@ kern_renameat(struct thread *td, int old
 	if (fromnd.ni_vp-v_type == VDIR)
 		tond.ni_cnd.cn_flags |= WILLBEDIR;
 	if ((error = namei(tond)) != 0) {
-		/* Translate error code for rename(dir1, dir2/.). */
-		if (error == EISDIR  fvp-v_type == VDIR)
-			error = EINVAL;
 		NDFREE(fromnd, NDF_ONLY_PNBUF);
 		vrele(fromnd.ni_dvp);
 		vrele(fvp);
Index: share/man/man9/namei.9
===
--- share/man/man9/namei.9	(revision 204620)
+++ share/man/man9/namei.9	(working copy)
@@ -338,8 +338,17 @@ An attempt is made to access a file in a
 permissions.
 .It Bq Er ELOOP
 Too many symbolic links were encountered in translating the pathname.
-.It Bq Er EISDIR
-An attempt is made to open a directory with write mode specified.
+.It Bq Er EINVAL
+The last component of the pathname specified for the RENAME operation is
+.Ql \. .
+.It Bq Er EEXIST
+The root directory
+.Pq Ql /
+was specified for the CREATE operation.
+.It Bq Er EBUSY
+The root directory
+.Pq Ql /
+was specified for the DELETE operation.
 .It Bq Er EROFS
 An attempt is made to modify a file or directory on a read-only file system.
 .El
Index: lib/libc/sys/rename.2
===
--- lib/libc/sys/rename.2	(revision 204620)
+++ lib/libc/sys/rename.2	(working copy)
@@ -252,6 +252,12 @@ The
 .Fa to
 argument
 is a directory and is not empty.
+.It Bq Er EBUSY
+Either
+.Fa from
+or
+.Fa to
+is the root directory.
 .El
 .Pp
 In addition to the errors returned by the
Index: lib/libc/sys/unlink.2
===
--- lib/libc/sys/unlink.2	(revision 204620)
+++ lib/libc/sys/unlink.2	(working copy)
@@ -114,8 +114,6 @@ succeeds unless:
 .Bl -tag -width Er
 .It Bq Er ENOTDIR
 A component of the path prefix is not a directory.
-.It Bq Er EISDIR
-The named file is a directory.
 .It Bq Er ENAMETOOLONG
 A component of a pathname exceeded 255 characters,
 or an entire path name exceeded 1023 characters.
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org

Re: namei() returns EISDIR for / (Re: svn commit: r203990 - head/lib/libc/sys)

2010-03-01 Thread Gary Jennejohn
On Sun, 28 Feb 2010 18:26:05 -0800
Garrett Cooper yanef...@gmail.com wrote:

 On Sun, Feb 28, 2010 at 5:11 PM, Alexander Best alexbes...@wwu.de wrote:
  i have a small test app to check {rm|mk}dir()'s errnos with certain args 
  like
  /, ., /proc and non-empty dirs. i'll submit it to this thread as soon as i
  also add testcases for syscalls like rename(), unlink(), etc.
 
  most of the errno codes returned after applying your patch look correct. i
  wonder however why rmdir(/proc) returns EACCESS as unprivileged user.
  wouldn't it make more sense to also return EBUSY? why complain about
  permission related matters when even root won't be able to perform the
  operation.
 
 Hmm.. good question. POSIX doesn't fully expound on this case
 (http://www.opengroup.org/onlinepubs/009695399/functions/rmdir.html),
 and either seem possible...
 

This maybe from rmdir(2)?

[EACCES]   Write permission is denied on the directory containing
   the link to be removed.

ls -ldo /
drwxr-xr-x  44 root  wheel  - 1536 Feb 28 18:36 /

ls -ldo /proc
dr-xr-xr-x  2 root  wheel  - 512 Sep  7  2008 /proc

---
Gary Jennejohn
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: namei() returns EISDIR for / (Re: svn commit: r203990 - head/lib/libc/sys)

2010-03-01 Thread Nate Eldredge

On Sun, 28 Feb 2010, Garrett Cooper wrote:


On Sun, Feb 28, 2010 at 5:11 PM, Alexander Best alexbes...@wwu.de wrote:

i have a small test app to check {rm|mk}dir()'s errnos with certain args like
/, ., /proc and non-empty dirs. i'll submit it to this thread as soon as i
also add testcases for syscalls like rename(), unlink(), etc.

most of the errno codes returned after applying your patch look correct. i
wonder however why rmdir(/proc) returns EACCESS as unprivileged user.
wouldn't it make more sense to also return EBUSY? why complain about
permission related matters when even root won't be able to perform the
operation.


Hmm.. good question. POSIX doesn't fully expound on this case
(http://www.opengroup.org/onlinepubs/009695399/functions/rmdir.html),
and either seem possible...


At:
http://www.opengroup.org/onlinepubs/009695399/functions/xsh_chap02_03.html#tag_02_03
we have

If more than one error occurs in processing a function call, any one of 
the possible errors may be returned, as the order of detection is 
undefined.


So we're okay standard-wise.

In general, though, I'd think it makes sense to do permissions checks 
before anything else, because in some cases the error code can leak 
information.  For instance, if you try to open() a nonexistent file in a 
directory for which you don't have search permission ('x' bit), it's very 
important that open() fail with EACCES instead of ENOENT, since you aren't 
suppposed to be able to find out whether or not the file exists. 
Obviously that doesn't apply in this case, because anyone is entitled to 
know that /proc is the root of a mounted filesystem, but it seems to me 
that it's a good habit to check permission first.


--

Nate Eldredge
n...@thatsmathematics.com
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


namei() returns EISDIR for / (Re: svn commit: r203990 - head/lib/libc/sys)

2010-02-28 Thread Jaakko Heinonen

[Moving discussion to -hackers. This is not directly related to the
original commit anymore.]

On 2010-02-26, Bruce Evans wrote:
  http://people.freebsd.org/~jh/patches/lookup-root.diff

 This is in relookup().  I think relookup() is only called from rename(),
 so the failing case is unreachable

I don't think it's true for unionfs. Looking at the code it seems that
it can call relookup() also with CREATE and DELETE nameiops.

 % @@ -3618,9 +3618,6 @@ kern_renameat(struct thread *td, int old
 % if (fromnd.ni_vp-v_type == VDIR)
 % tond.ni_cnd.cn_flags |= WILLBEDIR;
 % if ((error = namei(tond)) != 0) {
 % -   /* Translate error code for rename(dir1, dir2/.). */
 % -   if (error == EISDIR  fvp-v_type == VDIR)
 % -   error = EINVAL;
 
 I think this has nothing to do with the root directory (as its comment says),
 but it is to translate the EISDIR returned by ufs_lookup(), etc., when `tond'
 is for a (directory) pathname ending in ..  So it should not be removed,
 except possibly after changing ufs_lookup(), etc., to return EINVAL.  The
 EISDIR in ufs_lookup() is only for RENAME, so it is strange that any
 translation is needed.  I apparently put the translation here to avoid
 looking at all leaf file systems.

After r199137 the case of dir2/. is handled in lookup() before the
VOP_LOOKUP() call. I am not sure if it should be removed but it seems
that there's no need for the translation after r199137.

-- 
Jaakko
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: namei() returns EISDIR for / (Re: svn commit: r203990 - head/lib/libc/sys)

2010-02-28 Thread Alexander Best
Bruce Evans said that the doesn't like this comment you added in your patchset

/*
 * Check for  which is a way of talking about the root directory.
 * We can't provide a parent node for CREATE, DELETE and RENAME
 * operations.
 */

and would like to keep / the way it is instead of stripping the slash.
however this comment

/*
 * Replace multiple slashes by a single slash and trailing slashes
 * by a null.  This must be done before VOP_LOOKUP() because some
 * fs's don't know about trailing slashes.  Remember if there were
 * trailing slashes to handle symlinks, existing non-directories
 * and non-existing files that won't be directories specially later.
 */

says that some fs's can't handle trailing slashes. how exactly can the root
dir be expressed on these fs's? is it in fact ? and which fs's exactly are
lacking support for trailing slashes?

cheers.
alex
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: namei() returns EISDIR for / (Re: svn commit: r203990 - head/lib/libc/sys)

2010-02-28 Thread Alexander Best
i have a small test app to check {rm|mk}dir()'s errnos with certain args like
/, ., /proc and non-empty dirs. i'll submit it to this thread as soon as i
also add testcases for syscalls like rename(), unlink(), etc.

most of the errno codes returned after applying your patch look correct. i
wonder however why rmdir(/proc) returns EACCESS as unprivileged user.
wouldn't it make more sense to also return EBUSY? why complain about
permission related matters when even root won't be able to perform the
operation.

also: since namei() takes care of handling the / and . cases couldn't
those checks in kern_rmdirat() and kern_unlinkat() be removed?

it might also be interesting to look at the changes netbsd, openbsd and
dragonflybsd have made to vfs_lookup.c. just had a quick look at revision
1.121 from netbsd and they have split lookup() into loads of smaller functions
(see revision 1.117 and 1.118). seems they have been doing a lot of work here
(using heavy XXX-commenting however).

openbsd hasn't made to many changes to vfs_lookup.c.

matthew dillon seems to have done an incredible job on dragonfly in connection
with vfs_lookup.c. basically they completely got rid of namei() and are now
using nlookup() in commit ad57d0edbfceb0cebfb1dce61490df78fcc4a97. the commit
message is quite long and claims due to this change all syscalls which used to
call namei() have become a lot less complex after switching to nlookup().

right now vfs_lookup.c in dragonfly contains only some legacy code used for
compatibility.

again: an incredible job!

+1 for adapting those changes. ;)

cheers.
alex

btw.: there're a few NAMEI_DIAGNOSTIC ifdefs in vfs_lookup.c to increase
verbosity.
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org


Re: namei() returns EISDIR for / (Re: svn commit: r203990 - head/lib/libc/sys)

2010-02-28 Thread Garrett Cooper
On Sun, Feb 28, 2010 at 5:11 PM, Alexander Best alexbes...@wwu.de wrote:
 i have a small test app to check {rm|mk}dir()'s errnos with certain args like
 /, ., /proc and non-empty dirs. i'll submit it to this thread as soon as i
 also add testcases for syscalls like rename(), unlink(), etc.

 most of the errno codes returned after applying your patch look correct. i
 wonder however why rmdir(/proc) returns EACCESS as unprivileged user.
 wouldn't it make more sense to also return EBUSY? why complain about
 permission related matters when even root won't be able to perform the
 operation.

Hmm.. good question. POSIX doesn't fully expound on this case
(http://www.opengroup.org/onlinepubs/009695399/functions/rmdir.html),
and either seem possible...

 also: since namei() takes care of handling the / and . cases couldn't
 those checks in kern_rmdirat() and kern_unlinkat() be removed?

 it might also be interesting to look at the changes netbsd, openbsd and
 dragonflybsd have made to vfs_lookup.c. just had a quick look at revision
 1.121 from netbsd and they have split lookup() into loads of smaller functions
 (see revision 1.117 and 1.118). seems they have been doing a lot of work here
 (using heavy XXX-commenting however).

 openbsd hasn't made to many changes to vfs_lookup.c.

 matthew dillon seems to have done an incredible job on dragonfly in connection
 with vfs_lookup.c. basically they completely got rid of namei() and are now
 using nlookup() in commit ad57d0edbfceb0cebfb1dce61490df78fcc4a97. the commit
 message is quite long and claims due to this change all syscalls which used to
 call namei() have become a lot less complex after switching to nlookup().

 right now vfs_lookup.c in dragonfly contains only some legacy code used for
 compatibility.

 again: an incredible job!

 +1 for adapting those changes. ;)

 cheers.
 alex

 btw.: there're a few NAMEI_DIAGNOSTIC ifdefs in vfs_lookup.c to increase
 verbosity.

Thanks,
-Garrett
___
freebsd-hackers@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-hackers
To unsubscribe, send any mail to freebsd-hackers-unsubscr...@freebsd.org