Date: Fri, 06 Nov 2009 20:27:55 +0100 (CET) From: Alexander Best <alexbestms@math.uni-muenster.de> To: <gary.jennejohn@freenet.de> Cc: freebsd-hackers@FreeBSD.org Subject: Re: rmdir(2) and mkdir(2) both return EISDIR for argument "/" Message-ID: <permail-2009110619275580e26a0b00000d9c-a_best01@message-id.uni-muenster.de> In-Reply-To: <permail-200911061752401e86ffa8000045c7-a_best01@message-id.uni-muenster.de>
next in thread | previous in thread | raw e-mail | index | archive | help
Alexander Best schrieb am 2009-11-06: > Alexander Best schrieb am 2009-11-06: > > Gary Jennejohn schrieb am 2009-11-06: > > > On Fri, 06 Nov 2009 17:43:06 +0100 (CET) > > > Alexander Best <alexbestms@math.uni-muenster.de> wrote: > > > > Gary Jennejohn schrieb am 2009-11-06: > > > > > On Fri, 06 Nov 2009 16:32:22 +0100 (CET) > > > > > Alexander Best <alexbestms@math.uni-muenster.de> wrote: > > > > > > Alex Dupre schrieb am 2009-11-06: > > > > > > > Alexander Best ha scritto: > > > > > > > > i dug up this old pr > > > > > > > > http://www.freebsd.org/cgi/query-pr.cgi?pr=kern/59739 > > > > > > > I think the EISDIR error is coming from > > > > > > > kern/vfs_lookup.c, > > > > > > > lookup() > > > > > > > function with cn_nameptr = "": > > > > > > > /* > > > > > > > * Check for degenerate name (e.g. / or "") > > > > > > > * which is a way of talking about a directory, > > > > > > > * e.g. like "/." or ".". > > > > > > > */ > > > > > > > if (cnp->cn_nameptr[0] == '\0') { > > > > > > > ... > > > > > > > if (cnp->cn_nameiop != LOOKUP) { > > > > > > > error = EISDIR; > > > > > > > goto bad; > > > > > > > } > > > > > > > ... > > > > > > thanks a lot for finding the problem in the src. what do > > > > > > you > > > > > > think > > > > > > of the > > > > > > patch attached to this message? after applying it the > > > > > > example > > > > > > code > > > > > > i posted in > > > > > > my previous message returns the following output (instead > > > > > > of > > > > > > EISDIR): > > > > > > rmdir errno: 16 (which is EBUSY) > > > > > > mkdir errno: 17 (which is EEXIST) > > > > > > i don't know if these really are the correct return values, > > > > > > but > > > > > > it's what the > > > > > > originator of the PR requested. > > > > > What if cn_nameiop is != LOOKUP but also neither DELETE nor > > > > > CREATE, > > > > > assuming that case is possible? I'd leave the original > > > > > if-clause > > > > > at > > > > > the end to catch that. > > > > > --- > > > > > Gary Jennejohn > > > > how about this patch? > > > > 1. i've added "if (cnp->cn_nameiop != LOOKUP)" although i don't > > > > think it's > > > > necessary since the first blocks should cover all the possible > > > > cases. > > > > 2. i've used rename() to test the case (cnp->cn_nameiop != > > > > RENAME). > > > > is this > > > > correct or does rename() use a combo of DELETE and CREATE? > > > > problem > > > > is that the > > > > rename(2) manual doesn't seem to cover the case that arg 1 is a > > > > mountpoint. > > > > right now EBUSY gets returned if cnp->cn_nameiop != RENAME. > > > > however > > > > BUSY needs > > > > to be added to all manuals which use cnp->cn_nameiop != RENAME > > > > (shouldn't be > > > > too many). or are there any other suggestions what rename() > > > > should > > > > return if > > > > arg 1 is a mountpoint? > > > Hmm. In rename(2) there's > > > [EINVAL] The from argument is a parent directory of to, > > > or > > > an > > > attempt is made to rename `.' or `..'. > > > and a few lines below your patch this case is handled for > > > ISDOTDOT > > > for both RENAME and DELETE. I don't see off hand where renaming > > > or > > > deleting "." is handled. > > > According to the comment above your patch the case of "/." or "." > > > is > > > being checked, which would seem to correspond to the above part > > > of > > > rename(2), i.e. perhaps EINVAL should be returned for RENAME and > > > DELETE. > > > --- > > > Gary Jennejohn > > that would be an option. however in the case of rmdir(2) EINVAL and > > EBUYS > > would both fit. depends whether be forbid deletion of / because it > > is > > a > > mountpoint or because / is actually /. and paths ending with . are > > forbidden > > as arg in rmdir(2). > > i guess we have to take a look at the POSIX specs before we can > > decide how to > > handle this. > > also i've discovered that permission checks for / seem to be > > handled > > differently than any other dir. on my machine /usr is a mountpoint. > > doing > > rmdir /usr returns EACCES as regular user and EBUSY as superuser. > > doing rmdir > > / as regular user however doesn't seem to check permission but > > returns EBUSY > > right away. but that's not a problem i guess. this is probably > > happening > > because the kern/vfs_lookup.c code is being executed before > > anything > > else > > (including permission checks). > > i'll have a look what POSIX has to say about the return values. but > > i > > agree > > with you. returning EINVAL seems logical. > > alex. > ok. here it goes. POSIX says: > rmdir() > [http://www.opengroup.org/onlinepubs/9699919799/functions/rmdir.html#tag_16_618]: > If the directory is the root directory or the current working > directory of any > process, it is unspecified whether the function succeeds, or whether > it shall > fail and set errno to [EBUSY]. > mkdir() > [http://www.opengroup.org/onlinepubs/9699919799/functions/mkdir.html#tag_16_372]: > nothing regarding /. > rename() > [http://www.opengroup.org/onlinepubs/9699919799/functions/rename.html#tag_16_614]: > If either pathname argument refers to a path whose final component is > either > dot or dot-dot, rename() shall fail. > so at least rmdir() should return EBUSY. also POSIX defines EBUSY for > return() > which isn't documented in our return(2) manual. > alex this is getting a bit more complicated now. i've had a look at kern/vfs_syscalls.c where the basic rmdir(), rename() and mkdir() cals are being defined. looks like they themself set certain errno values, but those values get overwritten by kern/vfs_lookup.c i guess the right approach would be this one: if rmdir(), rename() or mkdir() is called with / set errno=EISDIR in kern/vfs_lookup.c. then in kern/vfs_syscalls.c do something like if(errno == EISDIR) return XXX; XXX being EBUSY for rmdir(), EINVAL for rename(), etc. i'll see if i can get this sorted out. alex
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?permail-2009110619275580e26a0b00000d9c-a_best01>