Summary of this email: found real bad commit and have a proposed fix (below) which works for me.
On May 14, 2011, at 4:40 PM, Arnd Bergmann wrote:
> On Saturday 14 May 2011, Erez Zadok wrote:
>>
>> Bisection narrowed the bug down to this one commit:
>
>> commit f74b9444192c60603020c61d7915b72893137edc
>> Merge: 7a63628 4ba8216
>> Author: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx>
>> Date: Wed Mar 16 17:21:00 2011 -0700
>>
>> Merge branch 'config' of git://git.kernel.org/pub/scm/linux/kernel/git/arnd/bkl
>
> As the above commit is a merge commit, it seems an unlikely candidate for causing the bug. That would
> indicate that something went wrong with Linus merging my patches, which I find less likely than
> a bug in one of my patches.
>
> Are you 100% sure that 4ba8216cd9 "BKL: That's all, folks" is actually working while the merged
> commit f74b94441 is broken?
>
> Arnd
Arnd, you're right. Maybe I messed up the bisection last night (swapped good/bad?). Today I tested the code before the above commit, and it was still buggy, so I went further back and bisected twice more. The first time gave me a patch related to symlink (getting warmer :-); I verified it manually by reversing that patch, and it still was wrong (either I made a mistake bisecting twice, or git-bisect can mess up some times). Anyway, third bisection round was a charm. This is the culprit:
----------------------------------------------------------
commit b356379a020bb7197603118bb1cbc903963aa198
Author: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
Date: Mon Mar 14 21:54:55 2011 -0400
Turn resolution of trailing symlinks iterative everywhere
The last remaining place (resolution of nested symlink) converted
to the loop of the same kind we have in path_lookupat() and
path_openat().
Note that we still *do* have a recursion in pathname resolution;
can't avoid it, really. However, it's strictly for nested symlinks
now - i.e. ones in the middle of a pathname.
link_path_walk() has lost the tail now - it always walks everything
except the last component.
do_follow_link() renamed to nested_symlink() and moved down.
Signed-off-by: Al Viro <viro@xxxxxxxxxxxxxxxxxx>
:040000 040000 3dec373b0f0021cb4f57cbb129d01ddfcab5400b e11d1f347ce72b76a377692d858d29d8a06c0e8e M fs
----------------------------------------------------------
I verified that the above commit was indeed causing the bug by going back and forth a bunch of times.
Next, I compared what the patch did. The only thing I could think of that has semantically changed was the order of checks for symlink depth. Old working code had this:
- if (current->link_count >= MAX_NESTED_LINKS)
- goto loop;
- if (current->total_link_count >= 40)
- goto loop;
- BUG_ON(nd->depth >= MAX_NESTED_LINKS);
New buggy code had the order reversed:
+ BUG_ON(nd->depth >= MAX_NESTED_LINKS);
+ if (unlikely(current->link_count >= MAX_NESTED_LINKS)) {
+ path_put_conditional(path, nd);
+ path_put(&nd->path);
+ return -ELOOP;
+ }
So I tried to move the BUG_ON below the test for ELOOP, and this solved the bug for me. I don't know if this is the right patch, Al, but I include it here for your review.
Cheers,
Erez.
——————————————————
VFS: move BUG_ON test for symlink nd->depth after current->link_count test
This solves a bug in nested_symlink (which was rewritten from
do_follow_link), and follows the order of depth tests that existed before.
The bug triggers a BUG_ON in fs/namei.c:1346, when running racer with
symlink and rename ops.
Signed-off-by: Erez Zadok <ezk@xxxxxxxxxxxxx>
diff --git a/fs/namei.c b/fs/namei.c
index 017c3fa..7a93387 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -1343,12 +1343,12 @@ static inline int nested_symlink(struct path *path, struct nameidata *nd)
{
int res;
- BUG_ON(nd->depth >= MAX_NESTED_LINKS);
if (unlikely(current->link_count >= MAX_NESTED_LINKS)) {
path_put_conditional(path, nd);
path_put(&nd->path);
return -ELOOP;
}
+ BUG_ON(nd->depth >= MAX_NESTED_LINKS);
nd->depth++;
current->link_count++;
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
