As most of you have noticed the problem only occured when trying
to load a skin.
Problem scenario:
x11amp created a directory in /tmp where it extracted the ziped skin.
After having read all the relevent files it wants to remove all the
files in the directory. To do this it does a getdents in the directory.
So far everything is fine, but next x11amp tries to do a
unlink("/tmp/03497aaa/.") where 03497aaa is obviously the temporary
directory it created to hold the skin files.
Nothing wrong with doing an unlink on ".", since it was returned in
getdents, but that the unlink call will return an error is quite obvious.
Here's where the problem starts.
Previously (kernel version < 2.1.132) unlink returned -EPERM
when trying to unlink a directory, which is not actually wrong.
To quote the unlink(2) manpage:
ERRORS
<snip>
EPERM The directory containing pathname has the sticky-
bit (S_ISVTX) set and the process's effective uid
is neither the uid of the file to be deleted nor
that of the directory containing it, or pathname
is a directory.
As we can see here returning -EPERM when trying to unlink a directory
is not wrong, but.. there's always a but, lets look further down
in the unlink(2) manpage:
EISDIR pathname refers to a directory.
Ahh, returning -EISDIR when trying to unlink a directory isn't wrong
either, actually, i personally feel its the right thing to do.
And this is what kernel 2.1.132 does, it now returns -EISDIR
instead of the former -EPERM when trying to unlink a directory.
from kernel 2.1.131 in linux/fs/namei.c
static inline int do_unlink(const char * name)
<code skipped>
/*
* A directory can't be unlink'ed.
* A file cannot be removed from an append-only directory.
*/
error = -EPERM;
if (S_ISDIR(dentry->d_inode->i_mode))
goto exit_lock;
As we can see above 2.1.131 returns -EPERM when trying to unlink a
directory.
Quite a few changes seem to have been made to the filesystem code
in 2.1.132, so lets trace the flow of trying to unlink a directory in
2.1.132.
from kernel 2.1.132 in linux/fs/namei.c
static inline int do_unlink(const char * name)
<code skipped>
error = vfs_unlink(dir->d_inode, dentry);
<jump>
int vfs_unlink(struct inode *dir, struct dentry *dentry)
<code skipped>
error = may_delete(dir, dentry, 0);
<jump>
static inline int may_delete(struct inode *dir,struct dentry *victim, int isdir)
<code skipped>
if (isdir) {
if (!S_ISDIR(victim->d_inode->i_mode))
return -ENOTDIR;
if (IS_ROOT(victim))
return -EBUSY;
if (victim->d_mounts != victim->d_covers)
return -EBUSY;
} else if (S_ISDIR(victim->d_inode->i_mode))
return -EISDIR;
The last two lines are the ones that are actually interesting in our
case i.e
} else if (S_ISDIR(victim->d_inode->i_mode))
return -EISDIR;
In other words.. 2.1.132 now returns -EISDIR when trying to unlink a
directory.
In x11amps case this is not good, let's see what happens there..
X11amp program flow with kernel 2.1.131:
getdents(9, /* 13 entries */, 3933) = 284
unlink("/tmp/00255aaa/.") = -1 EPERM (Operation not permitted)
unlink("/tmp/00255aaa/..") = -1 EACCES (Permission denied)
unlink("/tmp/00255aaa/main.bmp") = 0
<and so on>
We can see that here we get -EPERM when trying to unlink "." ie. a
directory. And x11amp continues trying to unlink the rest of the files.
x11amp program flow with kernel 2.1.132
getdents(9, /* 13 entries */, 3933) = 284
unlink("/tmp/03497aaa/.") = -1 EISDIR (Is a directory)
open("/tmp/03497aaa/.", O_RDONLY|O_NONBLOCK) = 10
<snip>
getdents(10, /* 13 entries */, 3933) = 284
unlink("/tmp/03497aaa/./.") = -1 EISDIR (Is a directory)
open("/tmp/03497aaa/./.", O_RDONLY|O_NONBLOCK) = 11
<snip>
getdents(11, /* 13 entries */, 3933) = 284
unlink("/tmp/03497aaa/././.") = -1 EISDIR (Is a directory)
open("/tmp/03497aaa/././.", O_RDONLY|O_NONBLOCK) = 12
<and so on for a very long time>
As we can see here trying to unlink "." returns -EISDIR.
X11amp obviously doesn't handle this error properly and goes into
a loop, adding a ./ for every failed unlink that returns -EISDIR.
(x11amp probably thinks there are subdirectories, thou thats just a
guess).
When x11amp has been going for a while it will finally get a
-EMFILE (The process has maximum number of files open)
when trying to do an open (since it hasnt been closing anything).
After that it starts trying to remove the directory, which of course
isn't empty since it hasn't removed the files yet.
After all this just about everything goes wrong for x11amp
and in the end it dies from a Segmentation fault caused by a write.
If anyone actually wants more details i can post them :)
Temporary solution for people who really feel they need skins in x11amp:
change line 546 in namei.c from
return -EISDIR;
to
return -EPERM;
I personally feel that -EISDIR is the correct error value, and that
x11amp needs to be patched.
But I'll leave it up to the "real big scary kernel coders" (Linus? Alan?)
to decide if trying to unlink a directory should return -EPERM or -EISDIR.
Sorry that this turned out so long, but i was bored, and its better
with to much info than to little, ehh? :)
Thanks
-Simon Ekstrand
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/