Re: kernel Ooops (2.5.63 bk latest)

From: Andrew Morton (akpm@digeo.com)
Date: Fri Feb 28 2003 - 02:34:34 EST


Maneesh Soni <maneesh@in.ibm.com> wrote:
>
> Hi Linus,
>
> The BUG was caught in d_validate() --> dget(). I think the
> dentry to be validated can be already on LRU list with d_count
> as zero. So, dget_locked should be used in place of dget().
> dcache_rcu mistakingly used dget. This patch corrects it.
>
> Please apply the following patch.
>
> diff -urN linux-2.5.63-bk3/fs/dcache.c linux-2.5.63-bk3-d_validate/fs/dcache.c
> --- linux-2.5.63-bk3/fs/dcache.c 2003-02-28 12:06:09.000000000 +0530
> +++ linux-2.5.63-bk3-d_validate/fs/dcache.c 2003-02-28 12:16:30.000000000 +0530
> @@ -1056,7 +1056,7 @@
> * as it is parsed under dcache_lock
> */
> if (dentry == list_entry(lhp, struct dentry, d_hash)) {
> - dget(dentry);
> + __dget_locked(dentry);
> spin_unlock(&dcache_lock);
> return 1;

Is this correct? If smbfs is playing around with dentries which are on
dentry_unused and which have a zero refcount then these can be freed up at
any time. The filesystem should have taken a ref on the dentry to prevent it
from being scavenged.

Isn't the bug over in smb_fill_cache(), which does:

        newdent = d_lookup(...);
        ...
        ctl.cache->dentry[ctl.idx] = newdent;
        ...
        dput(newdent);

I suspect we need to take an extra ref on the dentry when it is copied to the
cache, and put that ref back when smb_readdir() has finished using the dentry
(it looks like it's already doing that).

If so, the same problem is present in 2.4, but nobody noticed because 2.4 is
already using __dget_locked() and escapes the BUG check.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Fri Feb 28 2003 - 22:00:46 EST